From cd575023af846aa18ffa709f37bc70277e98cad3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Tue, 13 Aug 2019 12:35:04 +0200 Subject: Improve the server assets cache invalidation logic Fixes #6199 --- hugofs/glob/glob.go | 33 +++++--- hugofs/glob/glob_test.go | 26 ++++-- hugolib/site.go | 8 +- resources/resource_cache.go | 100 +++++++++++++++++++----- resources/resource_cache_test.go | 58 ++++++++++++++ resources/resource_factories/bundler/bundler.go | 3 +- resources/resource_factories/create/create.go | 23 +++--- resources/transform.go | 5 +- 8 files changed, 206 insertions(+), 50 deletions(-) create mode 100644 resources/resource_cache_test.go diff --git a/hugofs/glob/glob.go b/hugofs/glob/glob.go index 18d8d44cb..124a3d50e 100644 --- a/hugofs/glob/glob.go +++ b/hugofs/glob/glob.go @@ -51,7 +51,7 @@ func GetGlob(pattern string) (glob.Glob, error) { } func NormalizePath(p string) string { - return strings.Trim(filepath.ToSlash(strings.ToLower(p)), "/.") + return strings.Trim(path.Clean(filepath.ToSlash(strings.ToLower(p))), "/.") } // ResolveRootDir takes a normalized path on the form "assets/**.json" and @@ -60,14 +60,7 @@ func ResolveRootDir(p string) string { parts := strings.Split(path.Dir(p), "/") var roots []string for _, part := range parts { - isSpecial := false - for i := 0; i < len(part); i++ { - if syntax.Special(part[i]) { - isSpecial = true - break - } - } - if isSpecial { + if HasGlobChar(part) { break } roots = append(roots, part) @@ -79,3 +72,25 @@ func ResolveRootDir(p string) string { return strings.Join(roots, "/") } + +// FilterGlobParts removes any string with glob wildcard. +func FilterGlobParts(a []string) []string { + b := a[:0] + for _, x := range a { + if !HasGlobChar(x) { + b = append(b, x) + } + } + return b +} + +// HasGlobChar returns whether s contains any glob wildcards. +func HasGlobChar(s string) bool { + for i := 0; i < len(s); i++ { + if syntax.Special(s[i]) { + return true + } + } + return false + +} diff --git a/hugofs/glob/glob_test.go b/hugofs/glob/glob_test.go index 2b1c74107..cca8e4e0f 100644 --- a/hugofs/glob/glob_test.go +++ b/hugofs/glob/glob_test.go @@ -24,8 +24,8 @@ func TestResolveRootDir(t *testing.T) { c := qt.New(t) for _, test := range []struct { - in string - expect string + input string + expected string }{ {"data/foo.json", "data"}, {"a/b/**/foo.json", "a/b"}, @@ -33,7 +33,21 @@ func TestResolveRootDir(t *testing.T) { {"a/b[a-c]/foo.json", "a"}, } { - c.Assert(ResolveRootDir(test.in), qt.Equals, test.expect) + c.Assert(ResolveRootDir(test.input), qt.Equals, test.expected) + } +} + +func TestFilterGlobParts(t *testing.T) { + c := qt.New(t) + + for _, test := range []struct { + input []string + expected []string + }{ + {[]string{"a", "*", "c"}, []string{"a", "c"}}, + } { + + c.Assert(FilterGlobParts(test.input), qt.DeepEquals, test.expected) } } @@ -41,8 +55,8 @@ func TestNormalizePath(t *testing.T) { c := qt.New(t) for _, test := range []struct { - in string - expect string + input string + expected string }{ {filepath.FromSlash("data/FOO.json"), "data/foo.json"}, {filepath.FromSlash("/data/FOO.json"), "data/foo.json"}, @@ -50,7 +64,7 @@ func TestNormalizePath(t *testing.T) { {"//", ""}, } { - c.Assert(NormalizePath(test.in), qt.Equals, test.expect) + c.Assert(NormalizePath(test.input), qt.Equals, test.expected) } } diff --git a/hugolib/site.go b/hugolib/site.go index bf07d52b1..fb5dee46b 100644 --- a/hugolib/site.go +++ b/hugolib/site.go @@ -917,10 +917,12 @@ func (s *Site) processPartial(config *BuildCfg, init func(config *BuildCfg) erro logger = helpers.NewDistinctFeedbackLogger() ) - cachePartitions := make([]string, len(events)) + var cachePartitions []string - for i, ev := range events { - cachePartitions[i] = resources.ResourceKeyPartition(ev.Name) + for _, ev := range events { + if assetsFilename := s.BaseFs.Assets.MakePathRelative(ev.Name); assetsFilename != "" { + cachePartitions = append(cachePartitions, resources.ResourceKeyPartitions(assetsFilename)...) + } if s.isContentDirEvent(ev) { logger.Println("Source changed", ev) diff --git a/resources/resource_cache.go b/resources/resource_cache.go index 656d4f826..8f6fcbc0f 100644 --- a/resources/resource_cache.go +++ b/resources/resource_cache.go @@ -21,6 +21,10 @@ import ( "strings" "sync" + "github.com/gohugoio/hugo/helpers" + + "github.com/gohugoio/hugo/hugofs/glob" + "github.com/gohugoio/hugo/resources/resource" "github.com/gohugoio/hugo/cache/filecache" @@ -47,11 +51,14 @@ type ResourceCache struct { nlocker *locker.Locker } -// ResourceKeyPartition returns a partition name -// to allow for more fine grained cache flushes. -// It will return the file extension without the leading ".". If no -// extension, it will return "other". -func ResourceKeyPartition(filename string) string { +// ResourceCacheKey converts the filename into the format used in the resource +// cache. +func ResourceCacheKey(filename string) string { + filename = filepath.ToSlash(filename) + return path.Join(resourceKeyPartition(filename), filename) +} + +func resourceKeyPartition(filename string) string { ext := strings.TrimPrefix(path.Ext(filepath.ToSlash(filename)), ".") if ext == "" { ext = CACHE_OTHER @@ -59,6 +66,63 @@ func ResourceKeyPartition(filename string) string { return ext } +// Commonly used aliases and directory names used for some types. +var extAliasKeywords = map[string][]string{ + "sass": []string{"scss"}, + "scss": []string{"sass"}, +} + +// ResourceKeyPartitions resolves a ordered slice of partitions that is +// used to do resource cache invalidations. +// +// We use the first directory path element and the extension, so: +// a/b.json => "a", "json" +// b.json => "json" +// +// For some of the extensions we will also map to closely related types, +// e.g. "scss" will also return "sass". +// +func ResourceKeyPartitions(filename string) []string { + var partitions []string + filename = glob.NormalizePath(filename) + dir, name := path.Split(filename) + ext := strings.TrimPrefix(path.Ext(filepath.ToSlash(name)), ".") + + if dir != "" { + partitions = append(partitions, strings.Split(dir, "/")[0]) + } + + if ext != "" { + partitions = append(partitions, ext) + } + + if aliases, found := extAliasKeywords[ext]; found { + partitions = append(partitions, aliases...) + } + + if len(partitions) == 0 { + partitions = []string{CACHE_OTHER} + } + + return helpers.UniqueStringsSorted(partitions) +} + +// ResourceKeyContainsAny returns whether the key is a member of any of the +// given partitions. +// +// This is used for resource cache invalidation. +func ResourceKeyContainsAny(key string, partitions []string) bool { + parts := strings.Split(key, "/") + for _, p1 := range partitions { + for _, p2 := range parts { + if p1 == p2 { + return true + } + } + } + return false +} + func newResourceCache(rs *Spec) *ResourceCache { return &ResourceCache{ rs: rs, @@ -83,7 +147,7 @@ func (c *ResourceCache) Contains(key string) bool { } func (c *ResourceCache) cleanKey(key string) string { - return strings.TrimPrefix(path.Clean(key), "/") + return strings.TrimPrefix(path.Clean(strings.ToLower(key)), "/") } func (c *ResourceCache) get(key string) (interface{}, bool) { @@ -93,24 +157,24 @@ func (c *ResourceCache) get(key string) (interface{}, bool) { return r, found } -func (c *ResourceCache) GetOrCreate(partition, key string, f func() (resource.Resource, error)) (resource.Resource, error) { - r, err := c.getOrCreate(partition, key, func() (interface{}, error) { return f() }) +func (c *ResourceCache) GetOrCreate(key string, f func() (resource.Resource, error)) (resource.Resource, error) { + r, err := c.getOrCreate(key, func() (interface{}, error) { return f() }) if r == nil || err != nil { return nil, err } return r.(resource.Resource), nil } -func (c *ResourceCache) GetOrCreateResources(partition, key string, f func() (resource.Resources, error)) (resource.Resources, error) { - r, err := c.getOrCreate(partition, key, func() (interface{}, error) { return f() }) +func (c *ResourceCache) GetOrCreateResources(key string, f func() (resource.Resources, error)) (resource.Resources, error) { + r, err := c.getOrCreate(key, func() (interface{}, error) { return f() }) if r == nil || err != nil { return nil, err } return r.(resource.Resources), nil } -func (c *ResourceCache) getOrCreate(partition, key string, f func() (interface{}, error)) (interface{}, error) { - key = c.cleanKey(path.Join(partition, key)) +func (c *ResourceCache) getOrCreate(key string, f func() (interface{}, error)) (interface{}, error) { + key = c.cleanKey(key) // First check in-memory cache. r, found := c.get(key) if found { @@ -200,7 +264,7 @@ func (c *ResourceCache) set(key string, r interface{}) { func (c *ResourceCache) DeletePartitions(partitions ...string) { partitionsSet := map[string]bool{ - // Always clear out the resources not matching the partition. + // Always clear out the resources not matching any partition. "other": true, } for _, p := range partitions { @@ -217,13 +281,11 @@ func (c *ResourceCache) DeletePartitions(partitions ...string) { for k := range c.cache { clear := false - partIdx := strings.Index(k, "/") - if partIdx == -1 { - clear = true - } else { - partition := k[:partIdx] - if partitionsSet[partition] { + for p, _ := range partitionsSet { + if strings.Contains(k, p) { + // There will be some false positive, but that's fine. clear = true + break } } diff --git a/resources/resource_cache_test.go b/resources/resource_cache_test.go new file mode 100644 index 000000000..bcb241025 --- /dev/null +++ b/resources/resource_cache_test.go @@ -0,0 +1,58 @@ +// 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 resources + +import ( + "path/filepath" + "testing" + + qt "github.com/frankban/quicktest" +) + +func TestResourceKeyPartitions(t *testing.T) { + c := qt.New(t) + + for _, test := range []struct { + input string + expected []string + }{ + {"a.js", []string{"js"}}, + {"a.scss", []string{"sass", "scss"}}, + {"a.sass", []string{"sass", "scss"}}, + {"d/a.js", []string{"d", "js"}}, + {"js/a.js", []string{"js"}}, + {"D/a.JS", []string{"d", "js"}}, + {"d/a", []string{"d"}}, + {filepath.FromSlash("/d/a.js"), []string{"d", "js"}}, + {filepath.FromSlash("/d/e/a.js"), []string{"d", "js"}}, + } { + c.Assert(ResourceKeyPartitions(test.input), qt.DeepEquals, test.expected, qt.Commentf(test.input)) + } +} + +func TestResourceKeyContainsAny(t *testing.T) { + c := qt.New(t) + + for _, test := range []struct { + key string + filename string + expected bool + }{ + {"styles/css", "asdf.css", true}, + {"styles/css", "styles/asdf.scss", true}, + {"js/foo.bar", "asdf.css", false}, + } { + c.Assert(ResourceKeyContainsAny(test.key, ResourceKeyPartitions(test.filename)), qt.Equals, test.expected) + } +} diff --git a/resources/resource_factories/bundler/bundler.go b/resources/resource_factories/bundler/bundler.go index 6655ee5c3..c310efa33 100644 --- a/resources/resource_factories/bundler/bundler.go +++ b/resources/resource_factories/bundler/bundler.go @@ -18,6 +18,7 @@ import ( "bytes" "fmt" "io" + "path" "path/filepath" "github.com/gohugoio/hugo/common/hugio" @@ -66,7 +67,7 @@ func (r *multiReadSeekCloser) Close() error { // Concat concatenates the list of Resource objects. func (c *Client) Concat(targetPath string, r resource.Resources) (resource.Resource, error) { // The CACHE_OTHER will make sure this will be re-created and published on rebuilds. - return c.rs.ResourceCache.GetOrCreate(resources.CACHE_OTHER, targetPath, func() (resource.Resource, error) { + return c.rs.ResourceCache.GetOrCreate(path.Join(resources.CACHE_OTHER, targetPath), func() (resource.Resource, error) { var resolvedm media.Type // The given set of resources must be of the same Media Type. diff --git a/resources/resource_factories/create/create.go b/resources/resource_factories/create/create.go index e42843c75..23edf317e 100644 --- a/resources/resource_factories/create/create.go +++ b/resources/resource_factories/create/create.go @@ -18,6 +18,7 @@ package create import ( "path" "path/filepath" + "strings" "github.com/gohugoio/hugo/hugofs/glob" @@ -42,7 +43,7 @@ func New(rs *resources.Spec) *Client { // Get creates a new Resource by opening the given filename in the assets filesystem. func (c *Client) Get(filename string) (resource.Resource, error) { filename = filepath.Clean(filename) - return c.rs.ResourceCache.GetOrCreate(resources.ResourceKeyPartition(filename), filename, func() (resource.Resource, error) { + return c.rs.ResourceCache.GetOrCreate(resources.ResourceCacheKey(filename), func() (resource.Resource, error) { return c.rs.New(resources.ResourceSourceDescriptor{ Fs: c.rs.BaseFs.Assets.Fs, LazyPublish: true, @@ -66,18 +67,22 @@ func (c *Client) GetMatch(pattern string) (resource.Resource, error) { } func (c *Client) match(pattern string, firstOnly bool) (resource.Resources, error) { - var partition string + var name string if firstOnly { - partition = "__get-match" + name = "__get-match" } else { - partition = "__match" + name = "__match" } - // TODO(bep) match will be improved as part of https://github.com/gohugoio/hugo/issues/6199 - partition = path.Join(resources.CACHE_OTHER, partition) - key := glob.NormalizePath(pattern) + pattern = glob.NormalizePath(pattern) + partitions := glob.FilterGlobParts(strings.Split(pattern, "/")) + if len(partitions) == 0 { + partitions = []string{resources.CACHE_OTHER} + } + key := path.Join(name, path.Join(partitions...)) + key = path.Join(key, pattern) - return c.rs.ResourceCache.GetOrCreateResources(partition, key, func() (resource.Resources, error) { + return c.rs.ResourceCache.GetOrCreateResources(key, func() (resource.Resources, error) { var res resource.Resources handle := func(info hugofs.FileMetaInfo) (bool, error) { @@ -110,7 +115,7 @@ func (c *Client) match(pattern string, firstOnly bool) (resource.Resources, erro // FromString creates a new Resource from a string with the given relative target path. func (c *Client) FromString(targetPath, content string) (resource.Resource, error) { - return c.rs.ResourceCache.GetOrCreate(resources.CACHE_OTHER, targetPath, func() (resource.Resource, error) { + return c.rs.ResourceCache.GetOrCreate(path.Join(resources.CACHE_OTHER, targetPath), func() (resource.Resource, error) { return c.rs.New( resources.ResourceSourceDescriptor{ Fs: c.rs.FileCaches.AssetsCache().Fs, diff --git a/resources/transform.go b/resources/transform.go index 6a188e789..379452bb7 100644 --- a/resources/transform.go +++ b/resources/transform.go @@ -330,14 +330,13 @@ func (r *transformedResource) transform(setContent, publish bool) (err error) { if p == "" { panic("target path needed for key creation") } - partition := ResourceKeyPartition(p) - base = partition + "/" + p + base = ResourceCacheKey(p) default: return fmt.Errorf("transformation not supported for type %T", element) } } - key = r.cache.cleanKey(base + "_" + helpers.MD5String(key)) + key = r.cache.cleanKey(base) + "_" + helpers.MD5String(key) cached, found := r.cache.get(key) if found { -- cgit v1.2.3