Skip to content

Commit

Permalink
resources: Fix 2 image file cache key issues
Browse files Browse the repository at this point in the history
* Always include the content hash in the cache key for unprocessed images.
* Always include the image config hash in the cache key.

This is also a major cleanup/simplification of the implementation in this area.

Note that this, unfortunately, forces new hashes/filenames for generated images.

Fixes #13273
Fixes #13272
  • Loading branch information
bep committed Jan 18, 2025
1 parent 8897113 commit 1f5a15a
Show file tree
Hide file tree
Showing 38 changed files with 341 additions and 232 deletions.
2 changes: 1 addition & 1 deletion config/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
func DecodeNamespace[S, C any](configSource any, buildConfig func(any) (C, any, error)) (*ConfigNamespace[S, C], error) {
// Calculate the hash of the input (not including any defaults applied later).
// This allows us to introduce new config options without breaking the hash.
h := hashing.HashString(configSource)
h := hashing.HashStringHex(configSource)

// Build the config
c, ext, err := buildConfig(configSource)
Expand Down
2 changes: 1 addition & 1 deletion config/namespace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestNamespace(t *testing.T) {
c.Assert(err, qt.IsNil)
c.Assert(ns, qt.Not(qt.IsNil))
c.Assert(ns.SourceStructure, qt.DeepEquals, map[string]interface{}{"foo": "bar"})
c.Assert(ns.SourceHash, qt.Equals, "1450430416588600409")
c.Assert(ns.SourceHash, qt.Equals, "1420f6c7782f7459")
c.Assert(ns.Config, qt.DeepEquals, &tstNsExt{Foo: "bar"})
c.Assert(ns.Signature(), qt.DeepEquals, []*tstNsExt(nil))
}
Expand Down
4 changes: 2 additions & 2 deletions hugolib/hugo_sites_multihost_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,9 @@ title: mybundle-en
b.AssertFileExists("public/de/mybundle/pixel.png", true)
b.AssertFileExists("public/en/mybundle/pixel.png", true)

b.AssertFileExists("public/de/mybundle/pixel_hu8581513846771248023.png", true)
b.AssertFileExists("public/de/mybundle/pixel_hu_58204cbc58507d74.png", true)
// failing test below
b.AssertFileExists("public/en/mybundle/pixel_hu8581513846771248023.png", true)
b.AssertFileExists("public/en/mybundle/pixel_hu_58204cbc58507d74.png", true)
}

func TestMultihostResourceOneBaseURLWithSuPath(t *testing.T) {
Expand Down
16 changes: 8 additions & 8 deletions hugolib/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,20 +72,20 @@ SUNSET2: {{ $resized2.RelPermalink }}/{{ $resized2.Width }}/Lat: {{ $resized2.Ex

b.Build(BuildCfg{})

b.AssertFileContent("public/index.html", "SUNSET FOR: en: /bundle/sunset_hu13235715490294913361.jpg/200/Lat: 36.59744166666667")
b.AssertFileContent("public/fr/index.html", "SUNSET FOR: fr: /bundle/sunset_hu13235715490294913361.jpg/200/Lat: 36.59744166666667")
b.AssertFileContent("public/index.html", " SUNSET2: /images/sunset_hu1573057890424052540.jpg/123/Lat: 36.59744166666667")
b.AssertFileContent("public/nn/index.html", " SUNSET2: /images/sunset_hu1573057890424052540.jpg/123/Lat: 36.59744166666667")
b.AssertFileContent("public/index.html", "SUNSET FOR: en: /bundle/sunset_hu_77061c65c31d2244.jpg/200/Lat: 36.59744166666667")
b.AssertFileContent("public/fr/index.html", "SUNSET FOR: fr: /bundle/sunset_hu_77061c65c31d2244.jpg/200/Lat: 36.59744166666667")
b.AssertFileContent("public/index.html", " SUNSET2: /images/sunset_hu_b52e3343ea6a8764.jpg/123/Lat: 36.59744166666667")
b.AssertFileContent("public/nn/index.html", " SUNSET2: /images/sunset_hu_b52e3343ea6a8764.jpg/123/Lat: 36.59744166666667")

b.AssertImage(200, 200, "public/bundle/sunset_hu13235715490294913361.jpg")
b.AssertImage(200, 200, "public/bundle/sunset_hu_77061c65c31d2244.jpg")

// Check the file cache
b.AssertImage(200, 200, "resources/_gen/images/bundle/sunset_hu13235715490294913361.jpg")
b.AssertImage(200, 200, "resources/_gen/images/bundle/sunset_hu_77061c65c31d2244.jpg")

b.AssertFileContent("resources/_gen/images/bundle/sunset_17710516992648092201.json",
b.AssertFileContent("resources/_gen/images/bundle/sunset_d209dcdc6b875e26.json",
"FocalLengthIn35mmFormat|uint16", "PENTAX")

b.AssertFileContent("resources/_gen/images/images/sunset_17710516992648092201.json",
b.AssertFileContent("resources/_gen/images/images/sunset_d209dcdc6b875e26.json",
"FocalLengthIn35mmFormat|uint16", "PENTAX")

b.AssertNoDuplicateWrites()
Expand Down
2 changes: 1 addition & 1 deletion hugolib/pagesfromdata/pagesfromgotmpl_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ docs/p1/sub/mymixcasetext2.txt
"RelPermalink: /docs/p1/sub/mymixcasetext2.txt|Name: sub/mymixcasetext2.txt|",
"RelPermalink: /mydata.yaml|Name: sub/data1.yaml|Title: Sub data|Params: map[]|",
"Featured Image: /a/pixel.png|featured.png|",
"Resized Featured Image: /a/pixel_hu16809842526914527184.png|10|",
"Resized Featured Image: /a/pixel_hu_a32b3e361d55df1.png|10|",
// Resource from string
"RelPermalink: /docs/p1/mytext.txt|Name: textresource|Title: My Text Resource|Params: map[param1:param1v]|",
// Dates
Expand Down
4 changes: 2 additions & 2 deletions hugolib/resource_chain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,12 @@ FAILED REMOTE ERROR DETAILS CONTENT: {{ with $failedImg }}{{ with .Err }}{{ with
b.AssertFileContent("public/index.html",
fmt.Sprintf(`
SUNSET: /images/sunset.jpg|/images/sunset.a9bf1d944e19c0f382e0d8f51de690f7d0bc8fa97390c4242a86c3e5c0737e71.jpg|900|90587
FIT: /images/sunset.jpg|/images/sunset_hu15210517121918042184.jpg|200
FIT: /images/sunset.jpg|/images/sunset_hu_f2aae87288f3c13b.jpg|200
CSS integrity Data first: sha256-od9YaHw8nMOL8mUy97Sy8sKwMV3N4hI3aVmZXATxH+8= /styles.min.a1df58687c3c9cc38bf26532f7b4b2f2c2b0315dcde212376959995c04f11fef.css
CSS integrity Data last: /styles2.min.1cfc52986836405d37f9998a63fd6dd8608e8c410e5e3db1daaa30f78bc273ba.css sha256-HPxSmGg2QF03+ZmKY/1t2GCOjEEOXj2x2qow94vCc7o=
SUNSET REMOTE: /sunset_%[1]s.jpg|/sunset_%[1]s.a9bf1d944e19c0f382e0d8f51de690f7d0bc8fa97390c4242a86c3e5c0737e71.jpg|900|90587
FIT REMOTE: /sunset_%[1]s.jpg|/sunset_%[1]s_hu15210517121918042184.jpg|200
FIT REMOTE: /sunset_%[1]s.jpg|/sunset_%[1]s_hu_f2aae87288f3c13b.jpg|200
REMOTE NOT FOUND: OK
LOCAL NOT FOUND: OK
PRINT PROTOCOL ERROR DETAILS: Err: template: index.html:22:36: executing "index.html" at <resources.GetRemote>: error calling GetRemote: Get "gopher://example.org": unsupported protocol scheme "gopher"|
Expand Down
96 changes: 33 additions & 63 deletions resources/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (

"github.com/gohugoio/hugo/cache/filecache"
"github.com/gohugoio/hugo/common/hashing"
"github.com/gohugoio/hugo/common/hstrings"
"github.com/gohugoio/hugo/common/paths"

"github.com/disintegration/gift"
Expand Down Expand Up @@ -205,15 +204,12 @@ func (i *imageResource) cloneWithUpdates(u *transformationUpdate) (baseResource,
}, nil
}

var imageActions = []string{images.ActionResize, images.ActionCrop, images.ActionFit, images.ActionFill}

// Process processes the image with the given spec.
// The spec can contain an optional action, one of "resize", "crop", "fit" or "fill".
// This makes this method a more flexible version that covers all of Resize, Crop, Fit and Fill,
// but it also supports e.g. format conversions without any resize action.
func (i *imageResource) Process(spec string) (images.ImageResource, error) {
action, options := i.resolveActionOptions(spec)
return i.processActionOptions(action, options)
return i.processActionSpec("", spec)
}

// Resize resizes the image to the specified width and height using the specified resampling
Expand Down Expand Up @@ -243,55 +239,38 @@ func (i *imageResource) Fill(spec string) (images.ImageResource, error) {
}

func (i *imageResource) Filter(filters ...any) (images.ImageResource, error) {
var conf images.ImageConfig
var confMain images.ImageConfig

var gfilters []gift.Filter

for _, f := range filters {
gfilters = append(gfilters, images.ToFilters(f)...)
}

var (
targetFormat images.Format
configSet bool
)
var options []string

for _, f := range gfilters {
f = images.UnwrapFilter(f)
if specProvider, ok := f.(images.ImageProcessSpecProvider); ok {
action, options := i.resolveActionOptions(specProvider.ImageProcessSpec())
var err error
conf, err = images.DecodeImageConfig(action, options, i.Proc.Cfg, i.Format)
if err != nil {
return nil, err
}
configSet = true
if conf.TargetFormat != 0 {
targetFormat = conf.TargetFormat
// We only support one target format, but prefer the last one,
// so we keep going.
}
options = append(options, strings.Fields(specProvider.ImageProcessSpec())...)
}
}

if !configSet {
conf = images.GetDefaultImageConfig("filter", i.Proc.Cfg)
confMain, err := images.DecodeImageConfig(options, i.Proc.Cfg, i.Format)
if err != nil {
return nil, err
}

conf.Action = "filter"
conf.Key = hashing.HashString(gfilters)
conf.TargetFormat = targetFormat
if conf.TargetFormat == 0 {
conf.TargetFormat = i.Format
}
confMain.Action = "filter"
confMain.Key = hashing.HashString(gfilters)

return i.doWithImageConfig(conf, func(src image.Image) (image.Image, error) {
return i.doWithImageConfig(confMain, func(src image.Image) (image.Image, error) {
var filters []gift.Filter
for _, f := range gfilters {
f = images.UnwrapFilter(f)
if specProvider, ok := f.(images.ImageProcessSpecProvider); ok {
processSpec := specProvider.ImageProcessSpec()
action, options := i.resolveActionOptions(processSpec)
conf, err := images.DecodeImageConfig(action, options, i.Proc.Cfg, i.Format)
options := strings.Fields(specProvider.ImageProcessSpec())
conf, err := images.DecodeImageConfig(options, i.Proc.Cfg, i.Format)
if err != nil {
return nil, err
}
Expand All @@ -313,25 +292,13 @@ func (i *imageResource) Filter(filters ...any) (images.ImageResource, error) {
})
}

func (i *imageResource) resolveActionOptions(spec string) (string, []string) {
var action string
options := strings.Fields(spec)
for i, p := range options {
if hstrings.InSlicEqualFold(imageActions, p) {
action = p
options = append(options[:i], options[i+1:]...)
break
}
}
return action, options
}

func (i *imageResource) processActionSpec(action, spec string) (images.ImageResource, error) {
return i.processActionOptions(action, strings.Fields(spec))
options := append([]string{action}, strings.Fields(strings.ToLower(spec))...)
return i.processOptions(options)
}

func (i *imageResource) processActionOptions(action string, options []string) (images.ImageResource, error) {
conf, err := images.DecodeImageConfig(action, options, i.Proc.Cfg, i.Format)
func (i *imageResource) processOptions(options []string) (images.ImageResource, error) {
conf, err := images.DecodeImageConfig(options, i.Proc.Cfg, i.Format)
if err != nil {
return nil, err
}
Expand All @@ -343,13 +310,12 @@ func (i *imageResource) processActionOptions(action string, options []string) (i
return nil, err
}

if action == images.ActionFill {
if conf.Anchor == 0 && img.Width() == 0 || img.Height() == 0 {
if conf.Action == images.ActionFill {
if conf.Anchor == images.SmartCropAnchor && img.Width() == 0 || img.Height() == 0 {
// See https://github.com/gohugoio/hugo/issues/7955
// Smartcrop fails silently in some rare cases.
// Fall back to a center fill.
conf.Anchor = gift.CenterAnchor
conf.AnchorStr = "center"
conf = conf.Reanchor(gift.CenterAnchor)
return i.doWithImageConfig(conf, func(src image.Image) (image.Image, error) {
return i.Proc.ApplyFiltersFromConfig(src, conf)
})
Expand Down Expand Up @@ -417,7 +383,7 @@ func (i *imageResource) doWithImageConfig(conf images.ImageConfig, f func(src im
}

ci := i.clone(converted)
targetPath := i.relTargetPathFromConfig(conf)
targetPath := i.relTargetPathFromConfig(conf, i.getSpec().imaging.Cfg.SourceHash)
ci.setTargetPath(targetPath)
ci.Format = conf.TargetFormat
ci.setMediaType(conf.TargetFormat.MediaType())
Expand Down Expand Up @@ -485,26 +451,30 @@ func (i *imageResource) getImageMetaCacheTargetPath() string {
df := i.getResourcePaths()
p1, _ := paths.FileAndExt(df.File)
h := i.hash()
idStr := hashing.HashString(h, i.size(), imageMetaVersionNumber, cfgHash)
idStr := hashing.HashStringHex(h, i.size(), imageMetaVersionNumber, cfgHash)
df.File = fmt.Sprintf("%s_%s.json", p1, idStr)
return df.TargetPath()
}

func (i *imageResource) relTargetPathFromConfig(conf images.ImageConfig) internal.ResourcePaths {
func (i *imageResource) relTargetPathFromConfig(conf images.ImageConfig, imagingConfigSourceHash string) internal.ResourcePaths {
p1, p2 := paths.FileAndExt(i.getResourcePaths().File)
if conf.TargetFormat != i.Format {
p2 = conf.TargetFormat.DefaultExtension()
}
const prefix = "_hu"
huIdx := strings.LastIndex(p1, prefix)
incomingID := "i"

// Do not change.
const imageHashPrefix = "_hu_"

huIdx := strings.LastIndex(p1, imageHashPrefix)
incomingID := ""
if huIdx > -1 {
incomingID = p1[huIdx+len(prefix):]
incomingID = p1[huIdx+len(imageHashPrefix):]
p1 = p1[:huIdx]
}
hash := hashing.HashUint64(incomingID, i.hash(), conf.GetKey(i.Format))

hash := hashing.HashStringHex(incomingID, i.hash(), conf.Key, imagingConfigSourceHash)
rp := i.getResourcePaths()
rp.File = fmt.Sprintf("%s%s%d%s", p1, prefix, hash, p2)
rp.File = fmt.Sprintf("%s%s%s%s", p1, imageHashPrefix, hash, p2)

return rp
}
2 changes: 1 addition & 1 deletion resources/image_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (c *ImageCache) getOrCreate(
parent *imageResource, conf images.ImageConfig,
createImage func() (*imageResource, image.Image, error),
) (*resourceAdapter, error) {
relTarget := parent.relTargetPathFromConfig(conf)
relTarget := parent.relTargetPathFromConfig(conf, parent.getSpec().imaging.Cfg.SourceHash)
relTargetPath := relTarget.TargetPath()
memKey := relTargetPath

Expand Down
2 changes: 1 addition & 1 deletion resources/image_extended_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,6 @@ func TestImageResizeWebP(t *testing.T) {
resized, err := image.Resize("123x")
c.Assert(err, qt.IsNil)
c.Assert(image.MediaType(), qt.Equals, media.Builtin.WEBPType)
c.Assert(resized.RelPermalink(), qt.Equals, "/a/sunrise_hu544374262273649331.webp")
c.Assert(resized.RelPermalink(), qt.Equals, "/a/sunrise_hu_a1deb893888915d9.webp")
c.Assert(resized.Width(), qt.Equals, 123)
}
26 changes: 13 additions & 13 deletions resources/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,28 +113,28 @@ func TestImageTransformBasic(t *testing.T) {
assertWidthHeight(resizedAndRotated, 125, 200)

assertWidthHeight(resized, 300, 200)
c.Assert(resized.RelPermalink(), qt.Equals, "/a/sunset_hu2082030801149749592.jpg")
c.Assert(resized.RelPermalink(), qt.Equals, "/a/sunset_hu_d2115125d9324a79.jpg")

fitted, err := resized.Fit("50x50")
c.Assert(err, qt.IsNil)
c.Assert(fitted.RelPermalink(), qt.Equals, "/a/sunset_hu16263619592447877226.jpg")
c.Assert(fitted.RelPermalink(), qt.Equals, "/a/sunset_hu_c2c98e06123b048e.jpg")
assertWidthHeight(fitted, 50, 33)

// Check the MD5 key threshold
fittedAgain, _ := fitted.Fit("10x20")
fittedAgain, err = fittedAgain.Fit("10x20")
c.Assert(err, qt.IsNil)
c.Assert(fittedAgain.RelPermalink(), qt.Equals, "/a/sunset_hu847809310637164306.jpg")
c.Assert(fittedAgain.RelPermalink(), qt.Equals, "/a/sunset_hu_dc9e89c10109de72.jpg")
assertWidthHeight(fittedAgain, 10, 7)

filled, err := image.Fill("200x100 bottomLeft")
c.Assert(err, qt.IsNil)
c.Assert(filled.RelPermalink(), qt.Equals, "/a/sunset_hu18289448341423092707.jpg")
c.Assert(filled.RelPermalink(), qt.Equals, "/a/sunset_hu_b9f6d350738928fe.jpg")
assertWidthHeight(filled, 200, 100)

smart, err := image.Fill("200x100 smart")
c.Assert(err, qt.IsNil)
c.Assert(smart.RelPermalink(), qt.Equals, "/a/sunset_hu11649371610839769766.jpg")
c.Assert(smart.RelPermalink(), qt.Equals, "/a/sunset_hu_6fd390e7b0d26f0b.jpg")
assertWidthHeight(smart, 200, 100)

// Check cache
Expand All @@ -144,12 +144,12 @@ func TestImageTransformBasic(t *testing.T) {

cropped, err := image.Crop("300x300 topRight")
c.Assert(err, qt.IsNil)
c.Assert(cropped.RelPermalink(), qt.Equals, "/a/sunset_hu2242042514052853140.jpg")
c.Assert(cropped.RelPermalink(), qt.Equals, "/a/sunset_hu_3df036e11f4ddd43.jpg")
assertWidthHeight(cropped, 300, 300)

smartcropped, err := image.Crop("200x200 smart")
c.Assert(err, qt.IsNil)
c.Assert(smartcropped.RelPermalink(), qt.Equals, "/a/sunset_hu12983255101170993571.jpg")
c.Assert(smartcropped.RelPermalink(), qt.Equals, "/a/sunset_hu_12e2d26de89b464b.jpg")
assertWidthHeight(smartcropped, 200, 200)

// Check cache
Expand Down Expand Up @@ -216,15 +216,15 @@ func TestImageTransformFormat(t *testing.T) {

imagePng, err := image.Resize("450x png")
c.Assert(err, qt.IsNil)
c.Assert(imagePng.RelPermalink(), qt.Equals, "/a/sunset_hu11737890885216583918.png")
c.Assert(imagePng.RelPermalink(), qt.Equals, "/a/sunset_hu_e8b9444dcf2e75ef.png")
c.Assert(imagePng.ResourceType(), qt.Equals, "image")
assertExtWidthHeight(imagePng, ".png", 450, 281)
c.Assert(imagePng.Name(), qt.Equals, "sunset.jpg")
c.Assert(imagePng.MediaType().String(), qt.Equals, "image/png")

imageGif, err := image.Resize("225x gif")
c.Assert(err, qt.IsNil)
c.Assert(imageGif.RelPermalink(), qt.Equals, "/a/sunset_hu1431827106749674475.gif")
c.Assert(imageGif.RelPermalink(), qt.Equals, "/a/sunset_hu_f80842d4c3789345.gif")
c.Assert(imageGif.ResourceType(), qt.Equals, "image")
assertExtWidthHeight(imageGif, ".gif", 225, 141)
c.Assert(imageGif.Name(), qt.Equals, "sunset.jpg")
Expand All @@ -247,7 +247,7 @@ func TestImagePermalinkPublishOrder(t *testing.T) {
}()

check1 := func(img images.ImageResource) {
resizedLink := "/a/sunset_hu7919355342577096259.jpg"
resizedLink := "/a/sunset_hu_3910bca82e28c9d6.jpg"
c.Assert(img.RelPermalink(), qt.Equals, resizedLink)
assertImageFile(c, spec.PublishFs, resizedLink, 100, 50)
}
Expand Down Expand Up @@ -288,12 +288,12 @@ func TestImageBugs(t *testing.T) {
c.Assert(err, qt.IsNil)
c.Assert(resized, qt.Not(qt.IsNil))
c.Assert(resized.Width(), qt.Equals, 200)
c.Assert(resized.RelPermalink(), qt.Equals, "/a/1234567890qwertyuiopasdfghjklzxcvbnm5to6eeeeee7via8eleph_hu9514381480012510326.jpg")
c.Assert(resized.RelPermalink(), qt.Equals, "/a/1234567890qwertyuiopasdfghjklzxcvbnm5to6eeeeee7via8eleph_hu_951d3980b18c52a9.jpg")
resized, err = resized.Resize("100x")
c.Assert(err, qt.IsNil)
c.Assert(resized, qt.Not(qt.IsNil))
c.Assert(resized.Width(), qt.Equals, 100)
c.Assert(resized.RelPermalink(), qt.Equals, "/a/1234567890qwertyuiopasdfghjklzxcvbnm5to6eeeeee7via8eleph_hu1776700126481066216.jpg")
c.Assert(resized.RelPermalink(), qt.Equals, "/a/1234567890qwertyuiopasdfghjklzxcvbnm5to6eeeeee7via8eleph_hu_1daa203572ecd6ec.jpg")
})

// Issue #6137
Expand Down Expand Up @@ -391,7 +391,7 @@ func TestImageResize8BitPNG(t *testing.T) {
resized, err := image.Resize("800x")
c.Assert(err, qt.IsNil)
c.Assert(resized.MediaType().Type, qt.Equals, "image/png")
c.Assert(resized.RelPermalink(), qt.Equals, "/a/gohugoio_hu8582372628235034388.png")
c.Assert(resized.RelPermalink(), qt.Equals, "/a/gohugoio_hu_fe2b762e9cac406c.png")
c.Assert(resized.Width(), qt.Equals, 800)
}

Expand Down
Loading

0 comments on commit 1f5a15a

Please sign in to comment.