From 8624b9fe9eb81aeb884d36311fb6f85fed98aa43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Tue, 3 Sep 2019 10:36:09 +0200 Subject: [PATCH] Cache processed images by their source path Fixes #6269 --- hugolib/image_test.go | 86 ++++++++++++++++--- hugolib/testhelpers_test.go | 13 ++- resources/image.go | 6 +- resources/image_cache.go | 17 ++-- resources/image_test.go | 14 +-- resources/resource.go | 9 +- resources/resource_spec.go | 9 +- .../htesting/testhelpers.go | 2 +- resources/testhelpers_test.go | 4 +- 9 files changed, 127 insertions(+), 33 deletions(-) diff --git a/hugolib/image_test.go b/hugolib/image_test.go index 2ae1d85af..0cd781018 100644 --- a/hugolib/image_test.go +++ b/hugolib/image_test.go @@ -14,7 +14,6 @@ package hugolib import ( - "image/jpeg" "io" "os" "path/filepath" @@ -100,16 +99,6 @@ title: "My bundle" b := newBuilder() b.Build(BuildCfg{}) - assertImage := func(width, height int, filename string) { - filename = filepath.Join(workDir, "public", filename) - f, err := b.Fs.Destination.Open(filename) - c.Assert(err, qt.IsNil) - defer f.Close() - cfg, err := jpeg.DecodeConfig(f) - c.Assert(cfg.Width, qt.Equals, width) - c.Assert(cfg.Height, qt.Equals, height) - } - imgExpect := ` Resized1: images/sunset.jpg|123|234|image/jpg|/images/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_123x234_resize_q75_box.jpg| Resized2: images/sunset.jpg|12|23|image/jpg|/images/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_ada4bb1a57f77a63306e3bd67286248e.jpg| @@ -121,7 +110,7 @@ Resized6: images/sunset.jpg|350|219|image/jpg|/images/sunset_hu59e56ffff1bc1d8d1 ` b.AssertFileContent(filepath.Join(workDir, "public/index.html"), imgExpect) - assertImage(350, 219, "images/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_350x0_resize_q75_box.a86fe88d894e5db613f6aa8a80538fefc25b20fa24ba0d782c057adcef616f56.jpg") + b.AssertImage(350, 219, "public/images/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_350x0_resize_q75_box.a86fe88d894e5db613f6aa8a80538fefc25b20fa24ba0d782c057adcef616f56.jpg") // Build it again to make sure we read images from file cache. b = newBuilder() @@ -130,3 +119,76 @@ Resized6: images/sunset.jpg|350|219|image/jpg|/images/sunset_hu59e56ffff1bc1d8d1 b.AssertFileContent(filepath.Join(workDir, "public/index.html"), imgExpect) } + +func TestImageResizeMultilingual(t *testing.T) { + + b := newTestSitesBuilder(t).WithConfigFile("toml", ` +baseURL="https://example.org" +defaultContentLanguage = "en" + +[languages] +[languages.en] +title = "Title in English" +languageName = "English" +weight = 1 +[languages.nn] +languageName = "Nynorsk" +weight = 2 +title = "Tittel på nynorsk" +[languages.nb] +languageName = "Bokmål" +weight = 3 +title = "Tittel på bokmål" +[languages.fr] +languageName = "French" +weight = 4 +title = "French Title" + +`) + + pageContent := `--- +title: "Page" +--- +` + + b.WithContent("bundle/index.md", pageContent) + b.WithContent("bundle/index.nn.md", pageContent) + b.WithContent("bundle/index.fr.md", pageContent) + b.WithSunset("content/bundle/sunset.jpg") + b.WithSunset("assets/images/sunset.jpg") + b.WithTemplates("index.html", ` +{{ with (.Site.GetPage "bundle" ) }} +{{ $sunset := .Resources.GetMatch "sunset*" }} +{{ if $sunset }} +{{ $resized := $sunset.Resize "200x200" }} +SUNSET FOR: {{ $.Site.Language.Lang }}: {{ $resized.RelPermalink }}/{{ $resized.Width }}/Lat: {{ $resized.Exif.Lat }} +{{ end }} +{{ else }} +No bundle for {{ $.Site.Language.Lang }} +{{ end }} + +{{ $sunset2 := resources.Get "images/sunset.jpg" }} +{{ $resized2 := $sunset2.Resize "123x234" }} +SUNSET2: {{ $resized2.RelPermalink }}/{{ $resized2.Width }}/Lat: {{ $resized2.Exif.Lat }} + +`) + + b.Build(BuildCfg{}) + + b.AssertFileContent("public/index.html", "SUNSET FOR: en: /bundle/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_200x200_resize_q75_box.jpg/200/Lat: 36.59744166666667") + b.AssertFileContent("public/fr/index.html", "SUNSET FOR: fr: /fr/bundle/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_200x200_resize_q75_box.jpg/200/Lat: 36.59744166666667") + b.AssertFileContent("public/index.html", " SUNSET2: /images/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_123x234_resize_q75_box.jpg/123/Lat: 36.59744166666667") + b.AssertFileContent("public/nn/index.html", " SUNSET2: /images/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_123x234_resize_q75_box.jpg/123/Lat: 36.59744166666667") + + b.AssertImage(200, 200, "public/fr/bundle/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_200x200_resize_q75_box.jpg") + b.AssertImage(200, 200, "public/bundle/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_200x200_resize_q75_box.jpg") + + // Check the file cache + b.AssertImage(200, 200, "resources/_gen/images/bundle/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_200x200_resize_q75_box.jpg") + b.AssertFileContent("resources/_gen/images/bundle/sunset_17701188623491591036.json", + "DateTimeDigitized|time.Time", "PENTAX") + b.AssertImage(123, 234, "resources/_gen/images/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_123x234_resize_q75_box.jpg") + b.AssertFileContent("resources/_gen/images/sunset_17701188623491591036.json", + "DateTimeDigitized|time.Time", "PENTAX") + +} diff --git a/hugolib/testhelpers_test.go b/hugolib/testhelpers_test.go index f1c19366d..6e62a2442 100644 --- a/hugolib/testhelpers_test.go +++ b/hugolib/testhelpers_test.go @@ -1,6 +1,7 @@ package hugolib import ( + "image/jpeg" "io" "path/filepath" "runtime" @@ -334,7 +335,7 @@ func (s *sitesBuilder) WithSunset(in string) { src, err := os.Open(filepath.FromSlash("testdata/sunset.jpg")) s.Assert(err, qt.IsNil) - out, err := s.Fs.Source.Create(filepath.FromSlash(in)) + out, err := s.Fs.Source.Create(filepath.FromSlash(filepath.Join(s.workingDir, in))) s.Assert(err, qt.IsNil) _, err = io.Copy(out, src) @@ -669,6 +670,16 @@ func (s *sitesBuilder) AssertFileContent(filename string, matches ...string) { } } +func (s *sitesBuilder) AssertImage(width, height int, filename string) { + filename = filepath.Join(s.workingDir, filename) + f, err := s.Fs.Destination.Open(filename) + s.Assert(err, qt.IsNil) + defer f.Close() + cfg, err := jpeg.DecodeConfig(f) + s.Assert(cfg.Width, qt.Equals, width) + s.Assert(cfg.Height, qt.Equals, height) +} + func (s *sitesBuilder) FileContent(filename string) string { s.T.Helper() filename = filepath.FromSlash(filename) diff --git a/resources/image.go b/resources/image.go index 8242f7119..8a9e6fde4 100644 --- a/resources/image.go +++ b/resources/image.go @@ -24,6 +24,7 @@ import ( "io/ioutil" "os" "path" + "path/filepath" "strings" "sync" @@ -319,10 +320,13 @@ func (i *imageResource) getImageMetaCacheTargetPath() string { cfg := i.getSpec().imaging.Cfg df := i.getResourcePaths().relTargetDirFile + if fi := i.getFileInfo(); fi != nil { + df.dir = filepath.Dir(fi.Meta().Path()) + } p1, _ := helpers.FileAndExt(df.file) h, _ := i.hash() idStr := internal.HashString(h, i.size(), imageMetaVersionNumber, cfg) - return path.Join(df.dir, fmt.Sprintf("%s%s.json", p1, idStr)) + return path.Join(df.dir, fmt.Sprintf("%s_%s.json", p1, idStr)) } func (i *imageResource) relTargetPathFromConfig(conf images.ImageConfig) dirFile { diff --git a/resources/image_cache.go b/resources/image_cache.go index 3a9e3c2c5..9192a8e43 100644 --- a/resources/image_cache.go +++ b/resources/image_cache.go @@ -73,11 +73,18 @@ func (c *imageCache) getOrCreate( parent *imageResource, conf images.ImageConfig, createImage func() (*imageResource, image.Image, error)) (*resourceAdapter, error) { relTarget := parent.relTargetPathFromConfig(conf) - key := parent.relTargetPathForRel(relTarget.path(), false, false, false) + memKey := parent.relTargetPathForRel(relTarget.path(), false, false, false) + + // For the file cache we want to generate and store it once if possible. + fileKeyPath := relTarget + if fi := parent.root.getFileInfo(); fi != nil { + fileKeyPath.dir = filepath.ToSlash(filepath.Dir(fi.Meta().Path())) + } + fileKey := fileKeyPath.path() // First check the in-memory store, then the disk. c.mu.RLock() - cachedImage, found := c.store[key] + cachedImage, found := c.store[memKey] c.mu.RUnlock() if found { @@ -133,7 +140,7 @@ func (c *imageCache) getOrCreate( // but the count of processed image variations for this site. c.pathSpec.ProcessingStats.Incr(&c.pathSpec.ProcessingStats.ProcessedImages) - _, err := c.fileCache.ReadOrCreate(key, read, create) + _, err := c.fileCache.ReadOrCreate(fileKey, read, create) if err != nil { return nil, err } @@ -142,13 +149,13 @@ func (c *imageCache) getOrCreate( img.setSourceFs(c.fileCache.Fs) c.mu.Lock() - if cachedImage, found = c.store[key]; found { + if cachedImage, found = c.store[memKey]; found { c.mu.Unlock() return cachedImage, nil } imgAdapter := newResourceAdapter(parent.getSpec(), true, img) - c.store[key] = imgAdapter + c.store[memKey] = imgAdapter c.mu.Unlock() return imgAdapter, nil diff --git a/resources/image_test.go b/resources/image_test.go index 5afc4e36a..0146e8bdd 100644 --- a/resources/image_test.go +++ b/resources/image_test.go @@ -18,6 +18,7 @@ import ( "math/big" "math/rand" "os" + "path" "path/filepath" "regexp" "strconv" @@ -90,17 +91,16 @@ func TestImageTransformBasic(t *testing.T) { resized0x, err := image.Resize("x200") c.Assert(err, qt.IsNil) assertWidthHeight(resized0x, 320, 200) - assertFileCache(c, fileCache, resized0x.RelPermalink(), 320, 200) + assertFileCache(c, fileCache, path.Base(resized0x.RelPermalink()), 320, 200) resizedx0, err := image.Resize("200x") c.Assert(err, qt.IsNil) assertWidthHeight(resizedx0, 200, 125) - assertFileCache(c, fileCache, resizedx0.RelPermalink(), 200, 125) + assertFileCache(c, fileCache, path.Base(resizedx0.RelPermalink()), 200, 125) resizedAndRotated, err := image.Resize("x200 r90") c.Assert(err, qt.IsNil) assertWidthHeight(resizedAndRotated, 125, 200) - assertFileCache(c, fileCache, resizedAndRotated.RelPermalink(), 125, 200) assertWidthHeight(resized, 300, 200) c.Assert(resized.RelPermalink(), qt.Equals, "/a/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_300x200_resize_q68_linear.jpg") @@ -121,19 +121,16 @@ func TestImageTransformBasic(t *testing.T) { c.Assert(err, qt.IsNil) c.Assert(filled.RelPermalink(), qt.Equals, "/a/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_200x100_fill_q68_linear_bottomleft.jpg") assertWidthHeight(filled, 200, 100) - assertFileCache(c, fileCache, filled.RelPermalink(), 200, 100) smart, err := image.Fill("200x100 smart") c.Assert(err, qt.IsNil) c.Assert(smart.RelPermalink(), qt.Equals, fmt.Sprintf("/a/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_200x100_fill_q68_linear_smart%d.jpg", 1)) assertWidthHeight(smart, 200, 100) - assertFileCache(c, fileCache, smart.RelPermalink(), 200, 100) // Check cache filledAgain, err := image.Fill("200x100 bottomLeft") c.Assert(err, qt.IsNil) c.Assert(filled, eq, filledAgain) - assertFileCache(c, fileCache, filledAgain.RelPermalink(), 200, 100) } // https://github.com/gohugoio/hugo/issues/4261 @@ -294,7 +291,6 @@ func TestImageResizeInSubPath(t *testing.T) { c := qt.New(t) image := fetchImage(c, "sub/gohugoio2.png") - fileCache := image.(specProvider).getSpec().FileCaches.ImageCache().Fs c.Assert(image.MediaType(), eq, media.PNGType) c.Assert(image.RelPermalink(), qt.Equals, "/a/sub/gohugoio2.png") @@ -306,7 +302,6 @@ func TestImageResizeInSubPath(t *testing.T) { c.Assert(resized.RelPermalink(), qt.Equals, "/a/sub/gohugoio2_hu0e1b9e4a4be4d6f86c7b37b9ccce3fbc_73886_101x101_resize_linear_2.png") c.Assert(resized.Width(), qt.Equals, 101) - assertFileCache(c, fileCache, resized.RelPermalink(), 101, 101) publishedImageFilename := filepath.Clean(resized.RelPermalink()) spec := image.(specProvider).getSpec() @@ -321,7 +316,6 @@ func TestImageResizeInSubPath(t *testing.T) { c.Assert(err, qt.IsNil) c.Assert(resizedAgain.RelPermalink(), qt.Equals, "/a/sub/gohugoio2_hu0e1b9e4a4be4d6f86c7b37b9ccce3fbc_73886_101x101_resize_linear_2.png") c.Assert(resizedAgain.Width(), qt.Equals, 101) - assertFileCache(c, fileCache, resizedAgain.RelPermalink(), 101, 101) assertImageFile(c, image.(specProvider).getSpec().BaseFs.PublishFs, publishedImageFilename, 101, 101) } @@ -529,7 +523,7 @@ func TestImageOperationsGolden(t *testing.T) { return } - dir1 := filepath.Join(workDir, "resources/_gen/images/a") + dir1 := filepath.Join(workDir, "resources/_gen/images") dir2 := filepath.FromSlash("testdata/golden") // The two dirs above should now be the same. diff --git a/resources/resource.go b/resources/resource.go index 3859e6044..637f8e8fd 100644 --- a/resources/resource.go +++ b/resources/resource.go @@ -22,6 +22,8 @@ import ( "path/filepath" "sync" + "github.com/gohugoio/hugo/hugofs" + "github.com/gohugoio/hugo/media" "github.com/gohugoio/hugo/source" @@ -172,6 +174,7 @@ type fileInfo interface { getSourceFilename() string setSourceFilename(string) setSourceFs(afero.Fs) + getFileInfo() hugofs.FileMetaInfo hash() (string, error) size() int } @@ -537,7 +540,7 @@ type resourceFileInfo struct { // the path to the file on the real filesystem. sourceFilename string - fi os.FileInfo + fi hugofs.FileMetaInfo // A hash of the source content. Is only calculated in caching situations. h *resourceHash @@ -555,6 +558,10 @@ func (fi *resourceFileInfo) ReadSeekCloser() (hugio.ReadSeekCloser, error) { return f, nil } +func (fi *resourceFileInfo) getFileInfo() hugofs.FileMetaInfo { + return fi.fi +} + func (fi *resourceFileInfo) getSourceFilename() string { return fi.sourceFilename } diff --git a/resources/resource_spec.go b/resources/resource_spec.go index cd8d61470..a992df355 100644 --- a/resources/resource_spec.go +++ b/resources/resource_spec.go @@ -22,6 +22,8 @@ import ( "path/filepath" "strings" + "github.com/gohugoio/hugo/hugofs" + "github.com/gohugoio/hugo/helpers" "github.com/gohugoio/hugo/cache/filecache" @@ -194,8 +196,13 @@ func (r *Spec) newGenericResourceWithBase( relTargetDirFile: dirFile{dir: fpath, file: fname}, } + var fim hugofs.FileMetaInfo + if osFileInfo != nil { + fim = osFileInfo.(hugofs.FileMetaInfo) + } + gfi := &resourceFileInfo{ - fi: osFileInfo, + fi: fim, openReadSeekerCloser: openReadSeekerCloser, sourceFs: sourceFs, sourceFilename: sourceFilename, diff --git a/resources/resource_transformers/htesting/testhelpers.go b/resources/resource_transformers/htesting/testhelpers.go index eb664ed3a..4dfc9855a 100644 --- a/resources/resource_transformers/htesting/testhelpers.go +++ b/resources/resource_transformers/htesting/testhelpers.go @@ -39,7 +39,7 @@ func NewTestResourceSpec() (*resources.Spec, error) { cfg.Set("imaging", imagingCfg) - fs := hugofs.NewMem(cfg) + fs := hugofs.NewFrom(hugofs.NewBaseFileDecorator(afero.NewMemMapFs()), cfg) s, err := helpers.NewPathSpec(fs, cfg, nil) if err != nil { diff --git a/resources/testhelpers_test.go b/resources/testhelpers_test.go index 3e0725452..5fab0eca0 100644 --- a/resources/testhelpers_test.go +++ b/resources/testhelpers_test.go @@ -66,6 +66,8 @@ func newTestResourceSpec(desc specDescriptor) *Spec { afs = afero.NewMemMapFs() } + afs = hugofs.NewBaseFileDecorator(afs) + c := desc.c cfg := createTestCfg() @@ -118,7 +120,7 @@ func newTestResourceOsFs(c *qt.C) (*Spec, string) { cfg.Set("workingDir", workDir) - fs := hugofs.NewFrom(hugofs.Os, cfg) + fs := hugofs.NewFrom(hugofs.NewBaseFileDecorator(hugofs.Os), cfg) fs.Destination = &afero.MemMapFs{} s, err := helpers.NewPathSpec(fs, cfg, nil)