From 193c6f915c0730f925fdd9a34f327ab61d6d8147 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Wed, 22 Jan 2025 17:47:54 +0100 Subject: [PATCH] Make cascade front matter order deterministic Fixes #12594 --- common/hashing/hashing.go | 16 ++- common/maps/ordered.go | 117 ++++++++++++++++++++ common/maps/ordered_test.go | 74 +++++++++++++ config/allconfig/allconfig.go | 7 +- hugolib/cascade_test.go | 35 ++++++ hugolib/content_map_page.go | 12 +- hugolib/page__meta.go | 32 +++--- resources/page/page_matcher.go | 14 +-- resources/page/page_matcher_test.go | 12 +- resources/page/pagemeta/page_frontmatter.go | 6 +- 10 files changed, 277 insertions(+), 48 deletions(-) create mode 100644 common/maps/ordered.go create mode 100644 common/maps/ordered_test.go diff --git a/common/hashing/hashing.go b/common/hashing/hashing.go index 18ec8262358..e4535675833 100644 --- a/common/hashing/hashing.go +++ b/common/hashing/hashing.go @@ -123,16 +123,24 @@ func HashUint64(vs ...any) uint64 { o = elements } - hashOpts := getHashOpts() - defer putHashOpts(hashOpts) - - hash, err := hashstructure.Hash(o, hashOpts) + hash, err := Hash(o) if err != nil { panic(err) } return hash } +// Hash returns a hash from vs. +func Hash(vs ...any) (uint64, error) { + hashOpts := getHashOpts() + defer putHashOpts(hashOpts) + var v any = vs + if len(vs) == 1 { + v = vs[0] + } + return hashstructure.Hash(v, hashOpts) +} + type keyer interface { Key() string } diff --git a/common/maps/ordered.go b/common/maps/ordered.go new file mode 100644 index 00000000000..c85b12e8460 --- /dev/null +++ b/common/maps/ordered.go @@ -0,0 +1,117 @@ +// Copyright 2024 The Hugo Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package maps + +import ( + "github.com/gohugoio/hugo/common/hashing" +) + +// Ordered is a map that can be iterated in the order of insertion. +// Note that insertion order is not affected if a key is re-inserted into the map. +// This is not thread safe. +type Ordered[K comparable, T any] struct { + // The keys in the order they were added. + keys []K + // The values. + values map[K]T +} + +// NewOrdered creates a new Ordered map. +func NewOrdered[K comparable, T any]() *Ordered[K, T] { + return &Ordered[K, T]{values: make(map[K]T)} +} + +// Set sets the value for the given key. +// Note that insertion order is not affected if a key is re-inserted into the map. +func (m *Ordered[K, T]) Set(key K, value T) { + // Check if key already exists. + if _, found := m.values[key]; !found { + m.keys = append(m.keys, key) + } + m.values[key] = value +} + +// Get gets the value for the given key. +func (m *Ordered[K, T]) Get(key K) (T, bool) { + value, found := m.values[key] + return value, found +} + +// Delete deletes the value for the given key. +func (m *Ordered[K, T]) Delete(key K) { + delete(m.values, key) + for i, k := range m.keys { + if k == key { + m.keys = append(m.keys[:i], m.keys[i+1:]...) + break + } + } +} + +// Clone creates a shallow copy of the map. +// If m is nil, it returns nil. +func (m *Ordered[K, T]) Clone() *Ordered[K, T] { + if m == nil { + return nil + } + clone := NewOrdered[K, T]() + for _, k := range m.keys { + clone.Set(k, m.values[k]) + } + return clone +} + +// Keys returns the keys in the order they were added. +func (m *Ordered[K, T]) Keys() []K { + if m == nil { + return nil + } + return m.keys +} + +// Values returns the values in the order they were added. +func (m *Ordered[K, T]) Values() []T { + if m == nil { + return nil + } + var values []T + for _, k := range m.keys { + values = append(values, m.values[k]) + } + return values +} + +// Len returns the number of items in the map. +func (m *Ordered[K, T]) Len() int { + return len(m.keys) +} + +// Range calls f sequentially for each key and value present in the map. +// If f returns false, range stops the iteration. +// TODO(bep) replace with iter.Seq2 when we bump go Go 1.24. +func (m *Ordered[K, T]) Range(f func(key K, value T) bool) { + if m == nil { + return + } + for _, k := range m.keys { + if !f(k, m.values[k]) { + return + } + } +} + +// Hash calculates a hash from the values. +func (m *Ordered[K, T]) Hash() (uint64, error) { + return hashing.Hash(m.values) +} diff --git a/common/maps/ordered_test.go b/common/maps/ordered_test.go new file mode 100644 index 00000000000..48c32983047 --- /dev/null +++ b/common/maps/ordered_test.go @@ -0,0 +1,74 @@ +// Copyright 2024 The Hugo Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package maps + +import ( + "testing" + + qt "github.com/frankban/quicktest" +) + +func TestOrdered(t *testing.T) { + c := qt.New(t) + + m := NewOrdered[string, int]() + m.Set("a", 1) + m.Set("b", 2) + m.Set("c", 3) + + c.Assert(m.Keys(), qt.DeepEquals, []string{"a", "b", "c"}) + c.Assert(m.Values(), qt.DeepEquals, []int{1, 2, 3}) + + v, found := m.Get("b") + c.Assert(found, qt.Equals, true) + c.Assert(v, qt.Equals, 2) + + m.Set("b", 22) + c.Assert(m.Keys(), qt.DeepEquals, []string{"a", "b", "c"}) + c.Assert(m.Values(), qt.DeepEquals, []int{1, 22, 3}) + + m.Delete("b") + + c.Assert(m.Keys(), qt.DeepEquals, []string{"a", "c"}) + c.Assert(m.Values(), qt.DeepEquals, []int{1, 3}) +} + +func TestOrderedHash(t *testing.T) { + c := qt.New(t) + + m := NewOrdered[string, int]() + m.Set("a", 1) + m.Set("b", 2) + m.Set("c", 3) + + h1, err := m.Hash() + c.Assert(err, qt.IsNil) + + m.Set("d", 4) + + h2, err := m.Hash() + c.Assert(err, qt.IsNil) + + c.Assert(h1, qt.Not(qt.Equals), h2) + + m = NewOrdered[string, int]() + m.Set("b", 2) + m.Set("a", 1) + m.Set("c", 3) + + h3, err := m.Hash() + c.Assert(err, qt.IsNil) + // Order does not matter. + c.Assert(h1, qt.Equals, h3) +} diff --git a/config/allconfig/allconfig.go b/config/allconfig/allconfig.go index 96d10b3bdd7..3c2eddcaba3 100644 --- a/config/allconfig/allconfig.go +++ b/config/allconfig/allconfig.go @@ -143,7 +143,7 @@ type Config struct { // The cascade configuration section contains the top level front matter cascade configuration options, // a slice of page matcher and params to apply to those pages. - Cascade *config.ConfigNamespace[[]page.PageMatcherParamsConfig, map[page.PageMatcher]maps.Params] `mapstructure:"-"` + Cascade *config.ConfigNamespace[[]page.PageMatcherParamsConfig, *maps.Ordered[page.PageMatcher, maps.Params]] `mapstructure:"-"` // The segments defines segments for the site. Used for partial/segmented builds. Segments *config.ConfigNamespace[map[string]segments.SegmentConfig, segments.Segments] `mapstructure:"-"` @@ -766,9 +766,10 @@ type Configs struct { } func (c *Configs) Validate(logger loggers.Logger) error { - for p := range c.Base.Cascade.Config { + c.Base.Cascade.Config.Range(func(p page.PageMatcher, params maps.Params) bool { page.CheckCascadePattern(logger, p) - } + return true + }) return nil } diff --git a/hugolib/cascade_test.go b/hugolib/cascade_test.go index cbceaaed58a..47e53d9277f 100644 --- a/hugolib/cascade_test.go +++ b/hugolib/cascade_test.go @@ -841,3 +841,38 @@ title: p1 b.AssertFileExists("public/s1/index.html", false) b.AssertFileExists("public/s1/p1/index.html", false) } + +// Issue 12594. +func TestCascadeOrder(t *testing.T) { + t.Parallel() + + files := ` +-- hugo.toml -- +disableKinds = ['rss','sitemap','taxonomy','term', 'home'] +-- content/_index.md -- +--- +title: Home +cascade: +- _target: + path: "**" + params: + background: yosemite.jpg +- _target: + params: + background: goldenbridge.jpg +--- +-- content/p1.md -- +--- +title: p1 +--- +-- layouts/_default/single.html -- +Background: {{ .Params.background }}| +-- layouts/_default/list.html -- +{{ .Title }}| + ` + + for i := 0; i < 10; i++ { + b := Test(t, files) + b.AssertFileContent("public/p1/index.html", "Background: yosemite.jpg") + } +} diff --git a/hugolib/content_map_page.go b/hugolib/content_map_page.go index b48504203d7..6927562f127 100644 --- a/hugolib/content_map_page.go +++ b/hugolib/content_map_page.go @@ -1387,7 +1387,7 @@ func (sa *sitePagesAssembler) applyAggregates() error { } // Handle cascades first to get any default dates set. - var cascade map[page.PageMatcher]maps.Params + var cascade *maps.Ordered[page.PageMatcher, maps.Params] if keyPage == "" { // Home page gets it's cascade from the site config. cascade = sa.conf.Cascade.Config @@ -1399,7 +1399,7 @@ func (sa *sitePagesAssembler) applyAggregates() error { } else { _, data := pw.WalkContext.Data().LongestPrefix(keyPage) if data != nil { - cascade = data.(map[page.PageMatcher]maps.Params) + cascade = data.(*maps.Ordered[page.PageMatcher, maps.Params]) } } @@ -1481,11 +1481,11 @@ func (sa *sitePagesAssembler) applyAggregates() error { pageResource := rs.r.(*pageState) relPath := pageResource.m.pathInfo.BaseRel(pageBundle.m.pathInfo) pageResource.m.resourcePath = relPath - var cascade map[page.PageMatcher]maps.Params + var cascade *maps.Ordered[page.PageMatcher, maps.Params] // Apply cascade (if set) to the page. _, data := pw.WalkContext.Data().LongestPrefix(resourceKey) if data != nil { - cascade = data.(map[page.PageMatcher]maps.Params) + cascade = data.(*maps.Ordered[page.PageMatcher, maps.Params]) } if err := pageResource.setMetaPost(cascade); err != nil { return false, err @@ -1549,10 +1549,10 @@ func (sa *sitePagesAssembler) applyAggregatesToTaxonomiesAndTerms() error { const eventName = "dates" if p.Kind() == kinds.KindTerm { - var cascade map[page.PageMatcher]maps.Params + var cascade *maps.Ordered[page.PageMatcher, maps.Params] _, data := pw.WalkContext.Data().LongestPrefix(s) if data != nil { - cascade = data.(map[page.PageMatcher]maps.Params) + cascade = data.(*maps.Ordered[page.PageMatcher, maps.Params]) } if err := p.setMetaPost(cascade); err != nil { return false, err diff --git a/hugolib/page__meta.go b/hugolib/page__meta.go index 07d9d1c0e4e..38fe6db56db 100644 --- a/hugolib/page__meta.go +++ b/hugolib/page__meta.go @@ -87,8 +87,8 @@ type pageMetaParams struct { // These are only set in watch mode. datesOriginal pagemeta.Dates - paramsOriginal map[string]any // contains the original params as defined in the front matter. - cascadeOriginal map[page.PageMatcher]maps.Params // contains the original cascade as defined in the front matter. + paramsOriginal map[string]any // contains the original params as defined in the front matter. + cascadeOriginal *maps.Ordered[page.PageMatcher, maps.Params] // contains the original cascade as defined in the front matter. } // From page front matter. @@ -96,10 +96,10 @@ type pageMetaFrontMatter struct { configuredOutputFormats output.Formats // outputs defined in front matter. } -func (m *pageMetaParams) init(preserveOringal bool) { - if preserveOringal { +func (m *pageMetaParams) init(preserveOriginal bool) { + if preserveOriginal { m.paramsOriginal = xmaps.Clone[maps.Params](m.pageConfig.Params) - m.cascadeOriginal = xmaps.Clone[map[page.PageMatcher]maps.Params](m.pageConfig.CascadeCompiled) + m.cascadeOriginal = m.pageConfig.CascadeCompiled.Clone() } } @@ -306,22 +306,22 @@ func (p *pageMeta) setMetaPre(pi *contentParseInfo, logger loggers.Logger, conf return nil } -func (ps *pageState) setMetaPost(cascade map[page.PageMatcher]maps.Params) error { +func (ps *pageState) setMetaPost(cascade *maps.Ordered[page.PageMatcher, maps.Params]) error { ps.m.setMetaPostCount++ var cascadeHashPre uint64 if ps.m.setMetaPostCount > 1 { cascadeHashPre = hashing.HashUint64(ps.m.pageConfig.CascadeCompiled) - ps.m.pageConfig.CascadeCompiled = xmaps.Clone[map[page.PageMatcher]maps.Params](ps.m.cascadeOriginal) + ps.m.pageConfig.CascadeCompiled = ps.m.cascadeOriginal.Clone() } // Apply cascades first so they can be overridden later. if cascade != nil { if ps.m.pageConfig.CascadeCompiled != nil { - for k, v := range cascade { - vv, found := ps.m.pageConfig.CascadeCompiled[k] + cascade.Range(func(k page.PageMatcher, v maps.Params) bool { + vv, found := ps.m.pageConfig.CascadeCompiled.Get(k) if !found { - ps.m.pageConfig.CascadeCompiled[k] = v + ps.m.pageConfig.CascadeCompiled.Set(k, v) } else { // Merge for ck, cv := range v { @@ -330,7 +330,8 @@ func (ps *pageState) setMetaPost(cascade map[page.PageMatcher]maps.Params) error } } } - } + return true + }) cascade = ps.m.pageConfig.CascadeCompiled } else { ps.m.pageConfig.CascadeCompiled = cascade @@ -354,16 +355,17 @@ func (ps *pageState) setMetaPost(cascade map[page.PageMatcher]maps.Params) error } // Cascade is also applied to itself. - for m, v := range cascade { - if !m.Matches(ps) { - continue + cascade.Range(func(k page.PageMatcher, v maps.Params) bool { + if !k.Matches(ps) { + return true } for kk, vv := range v { if _, found := ps.m.pageConfig.Params[kk]; !found { ps.m.pageConfig.Params[kk] = vv } } - } + return true + }) if err := ps.setMetaPostParams(); err != nil { return err diff --git a/resources/page/page_matcher.go b/resources/page/page_matcher.go index f2075273af4..8155be99d98 100644 --- a/resources/page/page_matcher.go +++ b/resources/page/page_matcher.go @@ -104,9 +104,9 @@ func CheckCascadePattern(logger loggers.Logger, m PageMatcher) { } } -func DecodeCascadeConfig(logger loggers.Logger, in any) (*config.ConfigNamespace[[]PageMatcherParamsConfig, map[PageMatcher]maps.Params], error) { - buildConfig := func(in any) (map[PageMatcher]maps.Params, any, error) { - cascade := make(map[PageMatcher]maps.Params) +func DecodeCascadeConfig(logger loggers.Logger, in any) (*config.ConfigNamespace[[]PageMatcherParamsConfig, *maps.Ordered[PageMatcher, maps.Params]], error) { + buildConfig := func(in any) (*maps.Ordered[PageMatcher, maps.Params], any, error) { + cascade := maps.NewOrdered[PageMatcher, maps.Params]() if in == nil { return cascade, []map[string]any{}, nil } @@ -134,7 +134,7 @@ func DecodeCascadeConfig(logger loggers.Logger, in any) (*config.ConfigNamespace for _, cfg := range cfgs { m := cfg.Target CheckCascadePattern(logger, m) - c, found := cascade[m] + c, found := cascade.Get(m) if found { // Merge for k, v := range cfg.Params { @@ -143,18 +143,18 @@ func DecodeCascadeConfig(logger loggers.Logger, in any) (*config.ConfigNamespace } } } else { - cascade[m] = cfg.Params + cascade.Set(m, cfg.Params) } } return cascade, cfgs, nil } - return config.DecodeNamespace[[]PageMatcherParamsConfig](in, buildConfig) + return config.DecodeNamespace[[]PageMatcherParamsConfig, *maps.Ordered[PageMatcher, maps.Params]](in, buildConfig) } // DecodeCascade decodes in which could be either a map or a slice of maps. -func DecodeCascade(logger loggers.Logger, in any) (map[PageMatcher]maps.Params, error) { +func DecodeCascade(logger loggers.Logger, in any) (*maps.Ordered[PageMatcher, maps.Params], error) { conf, err := DecodeCascadeConfig(logger, in) if err != nil { return nil, err diff --git a/resources/page/page_matcher_test.go b/resources/page/page_matcher_test.go index dfb479d5ed9..bc072ce15a5 100644 --- a/resources/page/page_matcher_test.go +++ b/resources/page/page_matcher_test.go @@ -133,16 +133,8 @@ func TestDecodeCascadeConfig(t *testing.T) { c.Assert(err, qt.IsNil) c.Assert(got, qt.IsNotNil) - c.Assert(got.Config, qt.DeepEquals, - map[PageMatcher]maps.Params{ - {Path: "", Kind: "page", Lang: "", Environment: ""}: { - "b": "bv", - }, - {Path: "", Kind: "page", Lang: "", Environment: "production"}: { - "a": "av", - }, - }, - ) + c.Assert(got.Config.Keys(), qt.DeepEquals, []PageMatcher{{Kind: "page", Environment: "production"}, {Kind: "page"}}) + c.Assert(got.Config.Values(), qt.DeepEquals, []maps.Params{{"a": string("av")}, {"b": string("bv")}}) c.Assert(got.SourceStructure, qt.DeepEquals, []PageMatcherParamsConfig{ { Params: maps.Params{"a": string("av")}, diff --git a/resources/page/pagemeta/page_frontmatter.go b/resources/page/pagemeta/page_frontmatter.go index d5d61260981..21789909ec8 100644 --- a/resources/page/pagemeta/page_frontmatter.go +++ b/resources/page/pagemeta/page_frontmatter.go @@ -114,9 +114,9 @@ type PageConfig struct { Content Source // Compiled values. - CascadeCompiled map[page.PageMatcher]maps.Params - ContentMediaType media.Type `mapstructure:"-" json:"-"` - IsFromContentAdapter bool `mapstructure:"-" json:"-"` + CascadeCompiled *maps.Ordered[page.PageMatcher, maps.Params] `mapstructure:"-" json:"-"` + ContentMediaType media.Type `mapstructure:"-" json:"-"` + IsFromContentAdapter bool `mapstructure:"-" json:"-"` } var DefaultPageConfig = PageConfig{