Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resources: Fix 2 image file cache key issues #13274

Merged
merged 1 commit into from
Jan 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading