resource: Fix path duplication/flattening in processed images

Fixes #4502
Closes #4501

diff --git a/resource/image.go b/resource/image.go
index 7dae99333d626b27136cd3b6475b9f80203b2fe2..a1e0aac708a42a476dfb6915571ff68bb8de9339 100644 (file)
--- a/resource/image.go
+++ b/resource/image.go
@@ -209,7 +209,7 @@ type imageConfig struct {
 }
 
 func (i *Image) isJPEG() bool {
-       name := strings.ToLower(i.relTargetPath)
+       name := strings.ToLower(i.relTargetPath.file)
        return strings.HasSuffix(name, ".jpg") || strings.HasSuffix(name, ".jpeg")
 }
 
@@ -237,9 +237,7 @@ func (i *Image) doWithImageConfig(action, spec string, f func(src image.Image, c
                }
        }
 
-       key := i.relTargetPathForRel(i.filenameFromConfig(conf), false)
-
-       return i.spec.imageCache.getOrCreate(i, key, func(resourceCacheFilename string) (*Image, error) {
+       return i.spec.imageCache.getOrCreate(i, conf, func(resourceCacheFilename string) (*Image, error) {
                ci := i.clone()
 
                errOp := action
@@ -449,7 +447,6 @@ func (i *Image) decodeSource() (image.Image, error) {
 
 func (i *Image) copyToDestination(src string) error {
        var res error
-
        i.copyToDestinationInit.Do(func() {
                target := filepath.Join(i.absPublishDir, i.target())
 
@@ -498,7 +495,6 @@ func (i *Image) copyToDestination(src string) error {
 }
 
 func (i *Image) encodeToDestinations(img image.Image, conf imageConfig, resourceCacheFilename, filename string) error {
-
        target := filepath.Join(i.absPublishDir, filename)
 
        file1, err := i.spec.Fs.Destination.Create(target)
@@ -574,11 +570,12 @@ func (i *Image) clone() *Image {
 }
 
 func (i *Image) setBasePath(conf imageConfig) {
-       i.relTargetPath = i.filenameFromConfig(conf)
+       i.relTargetPath = i.relTargetPathFromConfig(conf)
 }
 
-func (i *Image) filenameFromConfig(conf imageConfig) string {
-       p1, p2 := helpers.FileAndExt(i.relTargetPath)
+func (i *Image) relTargetPathFromConfig(conf imageConfig) dirFile {
+       p1, p2 := helpers.FileAndExt(i.relTargetPath.file)
+
        idStr := fmt.Sprintf("_hu%s_%d", i.hash, i.osFileInfo.Size())
 
        // Do not change for no good reason.
@@ -605,7 +602,11 @@ func (i *Image) filenameFromConfig(conf imageConfig) string {
                idStr = ""
        }
 
-       return fmt.Sprintf("%s%s_%s%s", p1, idStr, key, p2)
+       return dirFile{
+               dir:  i.relTargetPath.dir,
+               file: fmt.Sprintf("%s%s_%s%s", p1, idStr, key, p2),
+       }
+
 }
 
 func decodeImaging(m map[string]interface{}) (Imaging, error) {

diff --git a/resource/image_cache.go b/resource/image_cache.go
index a1d41ec381abd5b1ff60ee2fde77115e8fa31a0d..e63989f24f1bb6e04c0ad50dc8b5b853254bd562 100644 (file)
--- a/resource/image_cache.go
+++ b/resource/image_cache.go
@@ -49,10 +49,17 @@ func (c *imageCache) deleteByPrefix(prefix string) {
        }
 }
 
+func (c *imageCache) clear() {
+       c.mu.Lock()
+       defer c.mu.Unlock()
+       c.store = make(map[string]*Image)
+}
+
 func (c *imageCache) getOrCreate(
-       parent *Image, key string, create func(resourceCacheFilename string) (*Image, error)) (*Image, error) {
+       parent *Image, conf imageConfig, create func(resourceCacheFilename string) (*Image, error)) (*Image, error) {
 
-       relTargetFilename := key
+       relTarget := parent.relTargetPathFromConfig(conf)
+       key := parent.relTargetPathForRel(relTarget.path(), false)
 
        if c.pathSpec.Language != nil {
                // Avoid do and store more work than needed. The language versions will in
@@ -89,7 +96,7 @@ func (c *imageCache) getOrCreate(
 
        if exists {
                img = parent.clone()
-               img.relTargetPath = relTargetFilename
+               img.relTargetPath.file = relTarget.file
                img.absSourceFilename = cacheFilename
        } else {
                img, err = create(cacheFilename)

diff --git a/resource/image_test.go b/resource/image_test.go
index 1e5b3d531d01f25cc8d6b5082b6d2d03a263e6a2..0111d0850fc920be8bc4eee8fcb20fd34b25d4ec 100644 (file)
--- a/resource/image_test.go
+++ b/resource/image_test.go
@@ -16,6 +16,7 @@ package resource
 import (
        "fmt"
        "math/rand"
+       "path/filepath"
        "strconv"
        "testing"
 
@@ -278,6 +279,39 @@ func TestImageResize8BitPNG(t *testing.T) {
 
 }
 
+func TestImageResizeInSubPath(t *testing.T) {
+
+       assert := require.New(t)
+
+       image := fetchImage(assert, "sub/gohugoio2.png")
+
+       assert.Equal(imaging.PNG, image.format)
+       assert.Equal("/a/sub/gohugoio2.png", image.RelPermalink())
+       assert.Equal("image", image.ResourceType())
+
+       resized, err := image.Resize("101x101")
+       assert.NoError(err)
+       assert.Equal(imaging.PNG, resized.format)
+       assert.Equal("/a/sub/gohugoio2_hu0e1b9e4a4be4d6f86c7b37b9ccce3fbc_73886_101x101_resize_linear_2.png", resized.RelPermalink())
+       assert.Equal(101, resized.Width())
+
+       assertFileCache(assert, image.spec.Fs, resized.RelPermalink(), 101, 101)
+       publishedImageFilename := filepath.Join("/public", resized.RelPermalink())
+       assertImageFile(assert, image.spec.Fs, publishedImageFilename, 101, 101)
+       assert.NoError(image.spec.Fs.Destination.Remove(publishedImageFilename))
+
+       // Cleare mem cache to simulate reading from the file cache.
+       resized.spec.imageCache.clear()
+
+       resizedAgain, err := image.Resize("101x101")
+       assert.NoError(err)
+       assert.Equal("/a/sub/gohugoio2_hu0e1b9e4a4be4d6f86c7b37b9ccce3fbc_73886_101x101_resize_linear_2.png", resizedAgain.RelPermalink())
+       assert.Equal(101, resizedAgain.Width())
+       assertFileCache(assert, image.spec.Fs, resizedAgain.RelPermalink(), 101, 101)
+       assertImageFile(assert, image.spec.Fs, publishedImageFilename, 101, 101)
+
+}
+
 func TestSVGImage(t *testing.T) {
        assert := require.New(t)
        spec := newTestResourceSpec(assert)

diff --git a/resource/resource.go b/resource/resource.go
index 828b03da1037328fb6b6bddf01452037633ea925..8627d93c4f66057a6f66573f6a4587f9201f86d8 100644 (file)
--- a/resource/resource.go
+++ b/resource/resource.go
@@ -283,7 +283,7 @@ func (r *Spec) newResource(
                }
        }
 
-       gr := r.newGenericResource(targetPathBuilder, fi, absPublishDir, absSourceFilename, filepath.ToSlash(relTargetFilename), mimeType)
+       gr := r.newGenericResource(targetPathBuilder, fi, absPublishDir, absSourceFilename, relTargetFilename, mimeType)
 
        if mimeType == "image" {
                ext := strings.ToLower(helpers.Ext(absSourceFilename))
@@ -343,10 +343,23 @@ func (r *Spec) CacheStats() string {
        return s
 }
 
+type dirFile struct {
+       // This is the directory component with Unix-style slashes.
+       dir string
+       // This is the file component.
+       file string
+}
+
+func (d dirFile) path() string {
+       return path.Join(d.dir, d.file)
+}
+
 // genericResource represents a generic linkable resource.
 type genericResource struct {
        // The relative path to this resource.
-       relTargetPath string
+       relTargetPath dirFile
+
+       file string
 
        // Base is set when the output format's path has a offset, e.g. for AMP.
        base string
@@ -366,11 +379,11 @@ type genericResource struct {
 }
 
 func (l *genericResource) Permalink() string {
-       return l.spec.PermalinkForBaseURL(l.relPermalinkForRel(l.relTargetPath, false), l.spec.BaseURL.String())
+       return l.spec.PermalinkForBaseURL(l.relPermalinkForRel(l.relTargetPath.path(), false), l.spec.BaseURL.String())
 }
 
 func (l *genericResource) RelPermalink() string {
-       return l.relPermalinkForRel(l.relTargetPath, true)
+       return l.relPermalinkForRel(l.relTargetPath.path(), true)
 }
 
 func (l *genericResource) Name() string {
@@ -551,11 +564,11 @@ func replaceResourcePlaceholders(in string, counter int) string {
 }
 
 func (l *genericResource) target() string {
-       target := l.relTargetPathForRel(l.relTargetPath, false)
+       target := l.relTargetPathForRel(l.relTargetPath.path(), false)
        if l.spec.PathSpec.Languages.IsMultihost() {
                target = path.Join(l.spec.PathSpec.Language.Lang, target)
        }
-       return target
+       return filepath.Clean(target)
 }
 
 func (r *Spec) newGenericResource(
@@ -566,12 +579,17 @@ func (r *Spec) newGenericResource(
        baseFilename,
        resourceType string) *genericResource {
 
+       // This value is used both to construct URLs and file paths, but start
+       // with a Unix-styled path.
+       baseFilename = filepath.ToSlash(baseFilename)
+       fpath, fname := path.Split(baseFilename)
+
        return &genericResource{
                targetPathBuilder: targetPathBuilder,
                osFileInfo:        osFileInfo,
                absPublishDir:     absPublishDir,
                absSourceFilename: absSourceFilename,
-               relTargetPath:     baseFilename,
+               relTargetPath:     dirFile{dir: fpath, file: fname},
                resourceType:      resourceType,
                spec:              r,
                params:            make(map[string]interface{}),

diff --git a/resource/resource_test.go b/resource/resource_test.go
index f4c652d43984a56cb7ff3039841f9f208912c6e4..396b404461344291bae3a78af2bf3ca757a5f809 100644 (file)
--- a/resource/resource_test.go
+++ b/resource/resource_test.go
@@ -93,7 +93,7 @@ func TestNewResourceFromFilenameSubPathInBaseURL(t *testing.T) {
        assert.Equal("/docs/a/b/logo.png", r.RelPermalink())
        assert.Equal("https://example.com/docs/a/b/logo.png", r.Permalink())
        img := r.(*Image)
-       assert.Equal("/a/b/logo.png", img.target())
+       assert.Equal(filepath.FromSlash("/a/b/logo.png"), img.target())
 
 }
 

diff --git a/resource/testdata/sub/gohugoio2.png b/resource/testdata/sub/gohugoio2.png
new file mode 100644 (file)
index 0000000..0591db9
Binary files /dev/null and b/resource/testdata/sub/gohugoio2.png differ

diff --git a/resource/testhelpers_test.go b/resource/testhelpers_test.go
index 89653ce11920651db6e92e074c256321939b449d..a9015ab2cd0f811986a4f5e032adb0487734d7a0 100644 (file)
--- a/resource/testhelpers_test.go
+++ b/resource/testhelpers_test.go
@@ -94,7 +94,7 @@ func fetchImageForSpec(spec *Spec, assert *require.Assertions, name string) *Ima
 }
 
 func fetchResourceForSpec(spec *Spec, assert *require.Assertions, name string) Resource {
-       src, err := os.Open("testdata/" + name)
+       src, err := os.Open(filepath.FromSlash("testdata/" + name))
        assert.NoError(err)
 
        workingDir := spec.Cfg.GetString("workingDir")
@@ -116,8 +116,9 @@ func fetchResourceForSpec(spec *Spec, assert *require.Assertions, name string) R
 
        return r
 }
-func assertFileCache(assert *require.Assertions, fs *hugofs.Fs, filename string, width, height int) {
-       f, err := fs.Source.Open(filepath.Join("/res/_gen/images", filename))
+
+func assertImageFile(assert *require.Assertions, fs *hugofs.Fs, filename string, width, height int) {
+       f, err := fs.Source.Open(filename)
        assert.NoError(err)
        defer f.Close()
 
@@ -128,6 +129,10 @@ func assertFileCache(assert *require.Assertions, fs *hugofs.Fs, filename string,
        assert.Equal(height, config.Height)
 }
 
+func assertFileCache(assert *require.Assertions, fs *hugofs.Fs, filename string, width, height int) {
+       assertImageFile(assert, fs, filepath.Join("/res/_gen/images", filename), width, height)
+}
+
 func writeSource(t testing.TB, fs *hugofs.Fs, filename, content string) {
        writeToFs(t, fs.Source, filename, content)
 }