resources/images: Make the image cache more robust
authorBjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Mon, 25 Nov 2019 11:49:04 +0000 (12:49 +0100)
committerBjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Mon, 25 Nov 2019 17:59:06 +0000 (18:59 +0100)
Also allow timeout to be set as a duration string, e.g. `30s`.

Fixes #6501

cache/filecache/filecache.go
cache/filecache/filecache_test.go
hugolib/config.go
hugolib/image_test.go
hugolib/page__per_output.go
hugolib/site.go
lazy/init.go
markup/goldmark/convert.go
resources/image.go
resources/image_cache.go
resources/images/image.go

index 3628300fb651d7a4e7147b35bb1d112bb6be1abe..37870dd5f0883ab4ba75ed233fd02508519090e3 100644 (file)
@@ -129,7 +129,7 @@ func (c *Cache) WriteCloser(id string) (ItemInfo, io.WriteCloser, error) {
 // If not found a new file is created and passed to create, which should close
 // it when done.
 func (c *Cache) ReadOrCreate(id string,
-       read func(info ItemInfo, r io.Reader) error,
+       read func(info ItemInfo, r io.ReadSeeker) error,
        create func(info ItemInfo, w io.WriteCloser) error) (info ItemInfo, err error) {
        id = cleanID(id)
 
index a4bf45fe020b04b39834461938ba8ab3a314065e..5a5dac9830e93cf04e01fcb862cf8b01262616c1 100644 (file)
@@ -250,9 +250,9 @@ func TestFileCacheReadOrCreateErrorInRead(t *testing.T) {
 
        var result string
 
-       rf := func(failLevel int) func(info ItemInfo, r io.Reader) error {
+       rf := func(failLevel int) func(info ItemInfo, r io.ReadSeeker) error {
 
-               return func(info ItemInfo, r io.Reader) error {
+               return func(info ItemInfo, r io.ReadSeeker) error {
                        if failLevel > 0 {
                                if failLevel > 1 {
                                        return ErrFatal
index 1bed9c0d9c8cfb7f15b62458da71a955f37c7488..e29d6ac9f92881e429bc26e1ecd8c4b527be2fc0 100644 (file)
@@ -620,7 +620,7 @@ func loadDefaultSettingsFor(v *viper.Viper) error {
        v.SetDefault("disableAliases", false)
        v.SetDefault("debug", false)
        v.SetDefault("disableFastRender", false)
-       v.SetDefault("timeout", 30000) // 30 seconds
+       v.SetDefault("timeout", "30s")
        v.SetDefault("enableInlineShortcodes", false)
 
        return nil
index d0bff75a2ffb75d58483433abc26a8e944900a7a..bcd715f5ccc0d3f38d05b063317cde26a50f7b87 100644 (file)
@@ -35,11 +35,12 @@ func TestImageOps(t *testing.T) {
        c.Assert(err, qt.IsNil)
        defer clean()
 
-       newBuilder := func() *sitesBuilder {
+       newBuilder := func(timeout string) *sitesBuilder {
 
                v := viper.New()
                v.Set("workingDir", workDir)
                v.Set("baseURL", "https://example.org")
+               v.Set("timeout", timeout)
 
                b := newTestSitesBuilder(t).WithWorkingDir(workDir)
                b.Fs = hugofs.NewDefault(v)
@@ -49,9 +50,17 @@ func TestImageOps(t *testing.T) {
 title: "My bundle"
 ---
 
+{{< imgproc >}}
+
 `)
 
-               b.WithTemplatesAdded("index.html", `
+               b.WithTemplatesAdded(
+                       "shortcodes/imgproc.html", `
+{{ $img := resources.Get "images/sunset.jpg" }}
+{{ $r := $img.Resize "129x239" }}
+IMG SHORTCODE: {{ $r.RelPermalink }}/{{ $r.Width }}
+`,
+                       "index.html", `
 {{ $p := .Site.GetPage "mybundle" }}
 {{ $img1 := resources.Get "images/sunset.jpg" }}
 {{ $img2 := $p.Resources.GetMatch "sunset.jpg" }}
@@ -83,7 +92,7 @@ BG3: {{ $blurryGrayscale3.RelPermalink }}/{{ $blurryGrayscale3.Width }}
 {{ $blurryGrayscale4 := $r.Filter $filters }}
 BG4: {{ $blurryGrayscale4.RelPermalink }}/{{ $blurryGrayscale4.Width }}
 
-
+{{ $p.Content }}
 
 `)
 
@@ -112,8 +121,8 @@ BG4: {{ $blurryGrayscale4.RelPermalink }}/{{ $blurryGrayscale4.Width }}
        out.Close()
        src.Close()
 
-       b := newBuilder()
-       b.Build(BuildCfg{})
+       // First build it with a very short timeout to trigger errors.
+       b := newBuilder("10ns")
 
        imgExpect := `
 Resized1: images/sunset.jpg|123|234|image/jpg|/images/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_123x234_resize_q75_box.jpg|
@@ -126,16 +135,35 @@ BG1: /images/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_2ae8bb993431ec1aec4
 BG2: /images/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_2ae8bb993431ec1aec40fe59927b46b4.jpg/123
 BG3: /images/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_ed7740a90b82802261c2fbdb98bc8082.jpg/123
 BG4: /images/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_ed7740a90b82802261c2fbdb98bc8082.jpg/123
+IMG SHORTCODE: /images/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_129x239_resize_q75_box.jpg/129
 `
 
-       b.AssertFileContent(filepath.Join(workDir, "public/index.html"), imgExpect)
-       b.AssertImage(350, 219, "public/images/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_350x0_resize_q75_box.a86fe88d894e5db613f6aa8a80538fefc25b20fa24ba0d782c057adcef616f56.jpg")
+       assertImages := func() {
+               b.Helper()
+               b.AssertFileContent(filepath.Join(workDir, "public/index.html"), imgExpect)
+               b.AssertImage(350, 219, "public/images/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_350x0_resize_q75_box.a86fe88d894e5db613f6aa8a80538fefc25b20fa24ba0d782c057adcef616f56.jpg")
+               b.AssertImage(129, 239, "public/images/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_129x239_resize_q75_box.jpg")
+       }
+
+       err = b.BuildE(BuildCfg{})
+       c.Assert(err, qt.Not(qt.IsNil))
+
+       b = newBuilder("30s")
+       b.Build(BuildCfg{})
+
+       assertImages()
+
+       // Truncate one image.
+       imgInCache := filepath.Join(workDir, "resources/_gen/images/sunset_hu59e56ffff1bc1d8d122b1403d34e039f_90587_ed7740a90b82802261c2fbdb98bc8082.jpg")
+       f, err := os.Create(imgInCache)
+       c.Assert(err, qt.IsNil)
+       f.Close()
 
        // Build it again to make sure we read images from file cache.
-       b = newBuilder()
+       b = newBuilder("30s")
        b.Build(BuildCfg{})
 
-       b.AssertFileContent(filepath.Join(workDir, "public/index.html"), imgExpect)
+       assertImages()
 
 }
 
index 59de10be3d4840f981f544c2c5cff7022df29aa7..bc2a0accc0481b6d82023839e3c2bc88dd19f93c 100644 (file)
@@ -180,7 +180,7 @@ func newPageContentOutput(p *pageState) func(f output.Format) (*pageContentOutpu
                needTimeout := !p.renderable || p.shortcodeState.hasShortcodes()
 
                if needTimeout {
-                       cp.initMain = parent.BranchdWithTimeout(p.s.siteCfg.timeout, func(ctx context.Context) (interface{}, error) {
+                       cp.initMain = parent.BranchWithTimeout(p.s.siteCfg.timeout, func(ctx context.Context) (interface{}, error) {
                                return nil, initContent()
                        })
                } else {
@@ -249,8 +249,10 @@ type pageContentOutput struct {
 }
 
 func (p *pageContentOutput) Content() (interface{}, error) {
-       p.p.s.initInit(p.initMain, p.p)
-       return p.content, nil
+       if p.p.s.initInit(p.initMain, p.p) {
+               return p.content, nil
+       }
+       return nil, nil
 }
 
 func (p *pageContentOutput) FuzzyWordCount() int {
index 0b45c4803d4d587f2569a52b38cc78fbbc573e07..4b4df64515a63f8e01360c39afa05cb9956929e4 100644 (file)
@@ -181,11 +181,12 @@ func (init *siteInit) Reset() {
        init.menus.Reset()
 }
 
-func (s *Site) initInit(init *lazy.Init, pctx pageContext) {
+func (s *Site) initInit(init *lazy.Init, pctx pageContext) bool {
        _, err := init.Do()
        if err != nil {
                s.h.FatalError(pctx.wrapError(err))
        }
+       return err == nil
 }
 
 func (s *Site) prepareInits() {
@@ -410,10 +411,23 @@ func newSite(cfg deps.DepsCfg) (*Site, error) {
                return nil, err
        }
 
+       timeout := 30 * time.Second
+       if cfg.Language.IsSet("timeout") {
+               switch v := cfg.Language.Get("timeout").(type) {
+               case int:
+                       timeout = time.Duration(v) * time.Millisecond
+               case string:
+                       d, err := time.ParseDuration(v)
+                       if err == nil {
+                               timeout = d
+                       }
+               }
+       }
+
        siteConfig := siteConfigHolder{
                sitemap:          config.DecodeSitemap(config.Sitemap{Priority: -1, Filename: "sitemap.xml"}, cfg.Language.GetStringMap("sitemap")),
                taxonomiesConfig: taxonomies,
-               timeout:          time.Duration(cfg.Language.GetInt("timeout")) * time.Millisecond,
+               timeout:          timeout,
                hasCJKLanguage:   cfg.Language.GetBool("hasCJKLanguage"),
                enableEmoji:      cfg.Language.Cfg.GetBool("enableEmoji"),
        }
index a54fda96afd6d259c5ca63d5517e34d5a8ab3c3e..2fef027cfb9190bad93b31059718a4d1356cfdf5 100644 (file)
@@ -64,7 +64,7 @@ func (ini *Init) Branch(initFn func() (interface{}, error)) *Init {
 }
 
 // BranchdWithTimeout is same as Branch, but with a timeout.
-func (ini *Init) BranchdWithTimeout(timeout time.Duration, f func(ctx context.Context) (interface{}, error)) *Init {
+func (ini *Init) BranchWithTimeout(timeout time.Duration, f func(ctx context.Context) (interface{}, error)) *Init {
        return ini.Branch(func() (interface{}, error) {
                return ini.withTimeout(timeout, f)
        })
index 9ce0b0b56f8b53c7aa0d1ca4d8c4daf629333528..cb9b24ff857a78aa0d9efe51342a1619c4b11ece 100644 (file)
@@ -158,7 +158,7 @@ func (c *goldmarkConverter) Convert(ctx converter.RenderContext) (result convert
                        name := fmt.Sprintf("goldmark_%s.txt", c.ctx.DocumentID)
                        filename := filepath.Join(dir, name)
                        afero.WriteFile(hugofs.Os, filename, ctx.Src, 07555)
-                       err = errors.Errorf("[BUG] goldmark: create an issue on GitHub attaching the file in: %s", filename)
+                       err = errors.Errorf("[BUG] goldmark: %s: create an issue on GitHub attaching the file in: %s", r, filename)
 
                }
        }()
index 1991e65f51104eb4ec4585284426a8085a72bc40..cdea8a2a7d67911f66e5e365df37658e96ed128a 100644 (file)
@@ -88,7 +88,7 @@ func (i *imageResource) getExif() (*exif.Exif, error) {
 
                key := i.getImageMetaCacheTargetPath()
 
-               read := func(info filecache.ItemInfo, r io.Reader) error {
+               read := func(info filecache.ItemInfo, r io.ReadSeeker) error {
                        meta := &imageMeta{}
                        data, err := ioutil.ReadAll(r)
                        if err != nil {
index 9192a8e43edfadf6ee8c4432838203cb728460f1..f57d73ede560b9833615f5d0e6a4e5309e1c3e50 100644 (file)
@@ -96,12 +96,18 @@ func (c *imageCache) getOrCreate(
        // These funcs are protected by a named lock.
        // read clones the parent to its new name and copies
        // the content to the destinations.
-       read := func(info filecache.ItemInfo, r io.Reader) error {
+       read := func(info filecache.ItemInfo, r io.ReadSeeker) error {
                img = parent.clone(nil)
                rp := img.getResourcePaths()
                rp.relTargetDirFile.file = relTarget.file
                img.setSourceFilename(info.Name)
 
+               if err := img.InitConfig(r); err != nil {
+                       return err
+               }
+
+               r.Seek(0, 0)
+
                w, err := img.openDestinationsForWriting()
                if err != nil {
                        return err
@@ -114,6 +120,7 @@ func (c *imageCache) getOrCreate(
 
                defer w.Close()
                _, err = io.Copy(w, r)
+
                return err
        }
 
index bac05ab708fa89f77d11494de211cf184c0d0909..62e9bb558697992bf1ad08ba080083026a1269ac 100644 (file)
@@ -123,6 +123,15 @@ func (i Image) WithSpec(s Spec) *Image {
        return &i
 }
 
+// InitConfig reads the image config from the given reader.
+func (i *Image) InitConfig(r io.Reader) error {
+       var err error
+       i.configInit.Do(func() {
+               i.config, _, err = image.DecodeConfig(r)
+       })
+       return err
+}
+
 func (i *Image) initConfig() error {
        var err error
        i.configInit.Do(func() {
@@ -130,10 +139,7 @@ func (i *Image) initConfig() error {
                        return
                }
 
-               var (
-                       f      hugio.ReadSeekCloser
-                       config image.Config
-               )
+               var f hugio.ReadSeekCloser
 
                f, err = i.Spec.ReadSeekCloser()
                if err != nil {
@@ -141,11 +147,7 @@ func (i *Image) initConfig() error {
                }
                defer f.Close()
 
-               config, _, err = image.DecodeConfig(f)
-               if err != nil {
-                       return
-               }
-               i.config = config
+               i.config, _, err = image.DecodeConfig(f)
        })
 
        if err != nil {