Skip to content

Commit

Permalink
resource: Fix path duplication/flattening in processed images
Browse files Browse the repository at this point in the history
  • Loading branch information
bep committed Mar 14, 2018
1 parent e427ba4 commit 15fcb7c
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 24 deletions.
21 changes: 11 additions & 10 deletions resource/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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())

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand All @@ -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) {
Expand Down
14 changes: 11 additions & 3 deletions resource/image_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,18 @@ 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.filePath(), false)

if c.pathSpec.Language != nil {
// Avoid do and store more work than needed. The language versions will in
Expand Down Expand Up @@ -89,7 +97,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)
Expand Down
34 changes: 34 additions & 0 deletions resource/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package resource
import (
"fmt"
"math/rand"
"path/filepath"
"strconv"
"testing"

Expand Down Expand Up @@ -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)
Expand Down
29 changes: 22 additions & 7 deletions resource/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,10 +343,23 @@ func (r *Spec) CacheStats() string {
return s
}

type dirFile struct {
// This is the directory component in filepath.Split.
dir string
// This is the file component in filepath.Split.
file string
}

func (d dirFile) filePath() string {
return filepath.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
Expand All @@ -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.filePath(), false), l.spec.BaseURL.String())
}

func (l *genericResource) RelPermalink() string {
return l.relPermalinkForRel(l.relTargetPath, true)
return l.relPermalinkForRel(l.relTargetPath.filePath(), true)
}

func (l *genericResource) Name() string {
Expand Down Expand Up @@ -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.filePath(), false)
if l.spec.PathSpec.Languages.IsMultihost() {
target = path.Join(l.spec.PathSpec.Language.Lang, target)
target = filepath.Join(l.spec.PathSpec.Language.Lang, target)
}
return target
return filepath.Clean(target)
}

func (r *Spec) newGenericResource(
Expand All @@ -566,12 +579,14 @@ func (r *Spec) newGenericResource(
baseFilename,
resourceType string) *genericResource {

fdir, fname := filepath.Split(baseFilename)

return &genericResource{
targetPathBuilder: targetPathBuilder,
osFileInfo: osFileInfo,
absPublishDir: absPublishDir,
absSourceFilename: absSourceFilename,
relTargetPath: baseFilename,
relTargetPath: dirFile{dir: fdir, file: fname},
resourceType: resourceType,
spec: r,
params: make(map[string]interface{}),
Expand Down
2 changes: 1 addition & 1 deletion resource/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())

}

Expand Down
Binary file added resource/testdata/sub/gohugoio2.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
11 changes: 8 additions & 3 deletions resource/testhelpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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()

Expand All @@ -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)
}
Expand Down

0 comments on commit 15fcb7c

Please sign in to comment.