Merge pull request #487 from gardener/feat/manifest-weight-ordering

feat: sort manifest node tree by frontmatter weight

Author: VelmiraS

pkg/manifest/manifest.go

@@ -429,6 +429,7 @@ func ResolveManifest(url string, r registry.Interface, additionalTransformations
 		validateTreeAfterManifestToNodeTree,
 		removeFileTreeNodes,
 		setDefaultProcessor,
+		resolveOrder,
 	)
 	if err != nil {
 		return nil, err

pkg/manifest/manifest_test.go

@@ -63,6 +63,7 @@ var _ = Describe("Manifest test", func() {
 		Entry("covering manifest use cases", "manifest"),
 		Entry("covering fileTree filtering", "fileTree_filtering"),
 		Entry("covering fileTree combined with manually added file", "filetree_with_manual_file"),
+		Entry("covering weight-based ordering", "weight_ordering"),
 	)
 
 	Describe("When there are dirs with frontmatter collision", func() {

pkg/manifest/order.go

@@ -0,0 +1,70 @@
+// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors
+//
+// SPDX-License-Identifier: Apache-2.0
+
+package manifest
+
+import (
+	"sort"
+
+	"github.com/gardener/docforge/pkg/registry"
+	"k8s.io/klog/v2"
+)
+
+// weightOf extracts the numeric weight from a node's frontmatter.
+// Returns (value, true) for int or float64 weight values; (0, false) otherwise.
+func weightOf(n *Node) (int, bool) {
+	if n.Frontmatter == nil {
+		return 0, false
+	}
+	raw, ok := n.Frontmatter["weight"]
+	if !ok || raw == nil {
+		return 0, false
+	}
+	switch v := raw.(type) {
+	case int:
+		return v, true
+	case float64:
+		return int(v), true
+	default:
+		return 0, false
+	}
+}
+
+// resolveOrder sorts node.Structure so that children with a weight frontmatter
+// field come first in ascending order, followed by unweighted children in their
+// original manifest order. Equal weights preserve manifest order (stable sort).
+func resolveOrder(node *Node, _ *Node, _ registry.Interface) (bool, error) {
+	if len(node.Structure) < 2 {
+		return false, nil
+	}
+
+	hasWeight := false
+	hasNoWeight := false
+	for _, child := range node.Structure {
+		if _, ok := weightOf(child); ok {
+			hasWeight = true
+		} else {
+			hasNoWeight = true
+		}
+	}
+	if hasWeight && hasNoWeight {
+		klog.Warningf("mixed weight ordering at %s: some children have 'weight' frontmatter and some do not", node.NodePath())
+	}
+
+	if !hasWeight {
+		return false, nil
+	}
+
+	sort.SliceStable(node.Structure, func(i, j int) bool {
+		wi, iHasWeight := weightOf(node.Structure[i])
+		wj, jHasWeight := weightOf(node.Structure[j])
+		if iHasWeight && jHasWeight {
+			return wi < wj
+		}
+		// weighted nodes sort before unweighted
+		return iHasWeight
+	})
+
+	return false, nil
+}

pkg/manifest/order_test.go

@@ -0,0 +1,164 @@
+// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors
+//
+// SPDX-License-Identifier: Apache-2.0
+
+package manifest
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func fileNames(nodes []*Node) []string {
+	out := make([]string, len(nodes))
+	for i, n := range nodes {
+		out[i] = n.File
+	}
+	return out
+}
+
+func makeWeightedNode(name string, weight interface{}) *Node {
+	n := &Node{
+		FileType: FileType{File: name},
+		Type:     "file",
+	}
+	if weight != nil {
+		n.Frontmatter = map[string]interface{}{"weight": weight}
+	}
+	return n
+}
+
+var _ = Describe("weightOf", func() {
+	It("returns 0, false for nil Frontmatter", func() {
+		n := &Node{}
+		w, ok := weightOf(n)
+		Expect(ok).To(BeFalse())
+		Expect(w).To(Equal(0))
+	})
+
+	It("returns 0, false when weight key is absent", func() {
+		n := &Node{Frontmatter: map[string]interface{}{"title": "foo"}}
+		w, ok := weightOf(n)
+		Expect(ok).To(BeFalse())
+		Expect(w).To(Equal(0))
+	})
+
+	It("returns int weight", func() {
+		n := &Node{Frontmatter: map[string]interface{}{"weight": 5}}
+		w, ok := weightOf(n)
+		Expect(ok).To(BeTrue())
+		Expect(w).To(Equal(5))
+	})
+
+	It("returns float64 weight converted to int", func() {
+		n := &Node{Frontmatter: map[string]interface{}{"weight": float64(7)}}
+		w, ok := weightOf(n)
+		Expect(ok).To(BeTrue())
+		Expect(w).To(Equal(7))
+	})
+
+	It("returns 0, false for string weight", func() {
+		n := &Node{Frontmatter: map[string]interface{}{"weight": "heavy"}}
+		w, ok := weightOf(n)
+		Expect(ok).To(BeFalse())
+		Expect(w).To(Equal(0))
+	})
+
+	It("returns 0, false for bool weight", func() {
+		n := &Node{Frontmatter: map[string]interface{}{"weight": true}}
+		w, ok := weightOf(n)
+		Expect(ok).To(BeFalse())
+		Expect(w).To(Equal(0))
+	})
+})
+
+var _ = Describe("resolveOrder", func() {
+	It("preserves manifest order when no children have weight", func() {
+		parent := &Node{DirType: DirType{Structure: []*Node{
+			makeWeightedNode("a", nil),
+			makeWeightedNode("b", nil),
+			makeWeightedNode("c", nil),
+		}}}
+		_, err := resolveOrder(parent, nil, nil)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(fileNames(parent.Structure)).To(Equal([]string{"a", "b", "c"}))
+	})
+
+	It("sorts all-weighted children ascending by weight", func() {
+		parent := &Node{DirType: DirType{Structure: []*Node{
+			makeWeightedNode("c", 30),
+			makeWeightedNode("a", 10),
+			makeWeightedNode("b", 20),
+		}}}
+		_, err := resolveOrder(parent, nil, nil)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(fileNames(parent.Structure)).To(Equal([]string{"a", "b", "c"}))
+	})
+
+	It("places weighted children before unweighted, preserving unweighted manifest order", func() {
+		parent := &Node{DirType: DirType{Structure: []*Node{
+			makeWeightedNode("u1", nil),
+			makeWeightedNode("w2", 20),
+			makeWeightedNode("u2", nil),
+			makeWeightedNode("w1", 10),
+		}}}
+		_, err := resolveOrder(parent, nil, nil)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(fileNames(parent.Structure)).To(Equal([]string{"w1", "w2", "u1", "u2"}))
+	})
+
+	It("is stable for equal weights, falling back to manifest order", func() {
+		parent := &Node{DirType: DirType{Structure: []*Node{
+			makeWeightedNode("first", 10),
+			makeWeightedNode("second", 10),
+			makeWeightedNode("third", 10),
+		}}}
+		_, err := resolveOrder(parent, nil, nil)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(fileNames(parent.Structure)).To(Equal([]string{"first", "second", "third"}))
+	})
+
+	It("accepts float64 weight as YAML often produces", func() {
+		parent := &Node{DirType: DirType{Structure: []*Node{
+			makeWeightedNode("b", float64(20)),
+			makeWeightedNode("a", float64(10)),
+		}}}
+		_, err := resolveOrder(parent, nil, nil)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(fileNames(parent.Structure)).To(Equal([]string{"a", "b"}))
+	})
+
+	It("treats string or bool weight as absent without panicking", func() {
+		parent := &Node{DirType: DirType{Structure: []*Node{
+			makeWeightedNode("a", "heavy"),
+			makeWeightedNode("b", true),
+			makeWeightedNode("c", nil),
+		}}}
+		Expect(func() {
+			_, _ = resolveOrder(parent, nil, nil)
+		}).ToNot(Panic())
+		Expect(fileNames(parent.Structure)).To(Equal([]string{"a", "b", "c"}))
+	})
+
+	It("does not panic with nil Frontmatter on children", func() {
+		parent := &Node{DirType: DirType{Structure: []*Node{
+			{FileType: FileType{File: "x"}, Type: "file"},
+			{FileType: FileType{File: "y"}, Type: "file"},
+		}}}
+		Expect(func() {
+			_, _ = resolveOrder(parent, nil, nil)
+		}).ToNot(Panic())
+	})
+
+	It("exits early and returns false when fewer than 2 children", func() {
+		single := &Node{DirType: DirType{Structure: []*Node{makeWeightedNode("only", 5)}}}
+		changed, err := resolveOrder(single, nil, nil)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(changed).To(BeFalse())
+
+		empty := &Node{}
+		changed, err = resolveOrder(empty, nil, nil)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(changed).To(BeFalse())
+	})
+})

pkg/manifest/tests/manifests/weight_ordering.yaml

@@ -0,0 +1,28 @@
+structure:
+- dir: third
+  frontmatter:
+    weight: 30
+  structure:
+  - file: ../contents/README.txt
+- dir: first
+  frontmatter:
+    weight: 10
+  structure:
+  - dir: nested-b
+    frontmatter:
+      weight: 20
+    structure:
+    - file: ../../contents/README.txt
+  - dir: nested-a
+    frontmatter:
+      weight: 10
+    structure:
+    - file: ../../contents/README.txt
+- dir: second
+  frontmatter:
+    weight: 20
+  structure:
+  - file: ../contents/README.txt
+- dir: unweighted
+  structure:
+  - file: ../contents/README.txt

pkg/manifest/tests/results/weight_ordering.yaml

@@ -0,0 +1,25 @@
+- file: README.txt
+  processor: downloader
+  type: file
+  source: https://github.com/gardener/docforge/blob/master/contents/README.txt
+  path: first/nested-a
+- file: README.txt
+  processor: downloader
+  type: file
+  source: https://github.com/gardener/docforge/blob/master/contents/README.txt
+  path: first/nested-b
+- file: README.txt
+  processor: downloader
+  type: file
+  source: https://github.com/gardener/docforge/blob/master/contents/README.txt
+  path: second
+- file: README.txt
+  processor: downloader
+  type: file
+  source: https://github.com/gardener/docforge/blob/master/contents/README.txt
+  path: third
+- file: README.txt
+  processor: downloader
+  type: file
+  source: https://github.com/gardener/docforge/blob/master/contents/README.txt
+  path: unweighted