diff --git a/collections/slice.go b/collections/slice.go index d1797ef..5a50619 100644 --- a/collections/slice.go +++ b/collections/slice.go @@ -128,17 +128,17 @@ func MatchItem(item string, patterns ...string) (matches, negated bool) { return true, false } + patterns = normalizeMatchPatterns(patterns) + if len(patterns) == 0 { + return false, false + } + slices.SortFunc(patterns, sortPatterns) //process negations first for _, p := range patterns { - pattern, err := url.QueryUnescape(strings.TrimSpace(p)) - if err != nil { - continue - } - - if strings.HasPrefix(pattern, "!") { - if matchPattern(item, strings.TrimPrefix(pattern, "!")) { + if strings.HasPrefix(p, "!") { + if matchPattern(item, strings.TrimPrefix(p, "!")) { return false, true } } @@ -146,12 +146,7 @@ func MatchItem(item string, patterns ...string) (matches, negated bool) { } // then normal filters - for _, p := range patterns { - pattern, err := url.QueryUnescape(strings.TrimSpace(p)) - if err != nil { - continue - } - + for _, pattern := range patterns { if matchPattern(item, pattern) { return true, false } @@ -159,7 +154,7 @@ func MatchItem(item string, patterns ...string) (matches, negated bool) { //nolint:gosimple //lint:ignore S1008 ... - if IsExclusionOnlyPatterns(patterns) { + if isExclusionOnly(patterns) { // If all the filters were exlusions, and none of the exclusions excluded the item, then it's a match return true, false } @@ -177,14 +172,14 @@ func MatchItems(item string, patterns ...string) bool { return true } - slices.SortFunc(patterns, sortPatterns) + patterns = normalizeMatchPatterns(patterns) + if len(patterns) == 0 { + return false + } - for _, p := range patterns { - pattern, err := url.QueryUnescape(strings.TrimSpace(p)) - if err != nil { - continue - } + slices.SortFunc(patterns, sortPatterns) + for _, pattern := range patterns { if strings.HasPrefix(pattern, "!") { if matchPattern(item, strings.TrimPrefix(pattern, "!")) { return false @@ -200,7 +195,7 @@ func MatchItems(item string, patterns ...string) bool { //nolint:gosimple //lint:ignore S1008 ... - if IsExclusionOnlyPatterns(patterns) { + if isExclusionOnly(patterns) { // If all the filters were exlusions, and none of the exclusions excluded the item, then it's a match return true } @@ -241,6 +236,26 @@ func matchPattern(item, pattern string) bool { return false } +func normalizeMatchPatterns(patterns []string) []string { + var normalized []string + for _, pattern := range patterns { + parts := strings.Split(pattern, ",") + for _, part := range parts { + part = strings.TrimSpace(part) + if part == "" && len(parts) > 1 { + continue + } + + decoded, err := url.QueryUnescape(part) + if err != nil { + continue + } + normalized = append(normalized, decoded) + } + } + return normalized +} + // sortPatterns defines the priority for sorting: // exclusions ("!") have higher priority than other patterns. func sortPatterns(a, b string) int { @@ -260,6 +275,14 @@ func sortPatterns(a, b string) int { } func IsExclusionOnlyPatterns(patterns []string) bool { + return isExclusionOnly(normalizeMatchPatterns(patterns)) +} + +// isExclusionOnly reports whether every pattern excludes, over patterns that +// normalizeMatchPatterns has already split and decoded. Normalizing again would +// split a decoded %2C as though it were a separator, turning one exclusion into +// an exclusion plus an inclusion. +func isExclusionOnly(patterns []string) bool { if len(patterns) == 0 { return false } diff --git a/collections/slice_test.go b/collections/slice_test.go index a8484c5..fc4c566 100644 --- a/collections/slice_test.go +++ b/collections/slice_test.go @@ -41,9 +41,38 @@ var _ = Describe("MatchItems", func() { Entry("case insensitive suffix wildcard", "Apple", []string{"*PLE"}, true), Entry("case insensitive glob", "Apple", []string{"*PPL*"}, true), Entry("case insensitive exclusion", "Apple", []string{"!apple"}, false), + Entry("comma-separated include after exclusion matches", "Item2", []string{"!Item,Item2"}, true), + Entry("comma-separated exclusion wins", "Item", []string{"!Item,Item2"}, false), + Entry("comma-separated wildcard exclusion wins", "Item99", []string{"!Item*,Other"}, false), + Entry("comma-separated wildcard exclusion allows include", "Other", []string{"!Item*,Other"}, true), + Entry("comma-separated values trim raw token whitespace", "Item2", []string{" !Item , Item2 "}, true), + Entry("comma-separated malformed URL token is skipped", "Item2", []string{"!Item,%zz"}, true), + Entry("URL encoded comma remains a literal pattern character", "A,B", []string{"A%2CB,C"}, true), + Entry("URL encoded comma in an exclusion is one non-matching pattern", "A", []string{"!A%2CB"}, true), + Entry("URL encoded comma in an exclusion still excludes what it names", "A,B", []string{"!A%2CB"}, false), ) }) +var _ = Describe("MatchItem", func() { + It("expands comma-separated include and exclude patterns", func() { + matches, negated := MatchItem("Item2", "!Item,Item2") + Expect(matches).To(BeTrue()) + Expect(negated).To(BeFalse()) + + matches, negated = MatchItem("Item", "!Item,Item2") + Expect(matches).To(BeFalse()) + Expect(negated).To(BeTrue()) + }) +}) + +var _ = Describe("IsExclusionOnlyPatterns", func() { + It("expands comma-separated patterns before checking exclusions", func() { + Expect(IsExclusionOnlyPatterns([]string{"!Item,!Item2"})).To(BeTrue()) + Expect(IsExclusionOnlyPatterns([]string{"!Item,Item2"})).To(BeFalse()) + Expect(IsExclusionOnlyPatterns([]string{"!Item,"})).To(BeTrue()) + }) +}) + var _ = Describe("Append", func() { It("should append string slices", func() { slices := [][]any{ diff --git a/go.mod b/go.mod index fd5d578..71a2a45 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/flanksource/commons go 1.25.1 require ( + dario.cat/mergo v1.0.2 github.com/aws/aws-sdk-go-v2 v1.41.1 github.com/aws/aws-sdk-go-v2/config v1.32.9 github.com/aws/aws-sdk-go-v2/credentials v1.19.9 diff --git a/go.sum b/go.sum index e10e906..9eca91a 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,7 @@ cel.dev/expr v0.25.1 h1:1KrZg61W6TWSxuNZ37Xy49ps13NUovb66QLprthtwi4= cel.dev/expr v0.25.1/go.mod h1:hrXvqGP6G6gyx8UAHSHJ5RGk//1Oj5nXQ2NI02Nrsg4= +dario.cat/mergo v1.0.2 h1:85+piFYR1tMbRrLcDwR18y4UKJ3aH1Tbzi24VRW1TK8= +dario.cat/mergo v1.0.2/go.mod h1:E/hbnu0NxMFBjpMIE34DRGLWqDy0g5FuKDhCb31ngxA= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/Masterminds/goutils v1.1.1 h1:5nUrii3FMTL5diU80unEVvNevw1nH4+ZV4DSLVJLSYI= github.com/Masterminds/goutils v1.1.1/go.mod h1:8cTjp+g8YejhMuvIA5y2vz3BpJxksy863GQaJW2MFNU= @@ -255,8 +257,6 @@ github.com/prometheus/common v0.67.5 h1:pIgK94WWlQt1WLwAC5j2ynLaBRDiinoAb86HZHTU github.com/prometheus/common v0.67.5/go.mod h1:SjE/0MzDEEAyrdr5Gqc6G+sXI67maCxzaT3A2+HqjUw= github.com/prometheus/procfs v0.20.1 h1:XwbrGOIplXW/AU3YhIhLODXMJYyC1isLFfYCsTEycfc= github.com/prometheus/procfs v0.20.1/go.mod h1:o9EMBZGRyvDrSPH1RqdxhojkuXstoe4UlK79eF5TGGo= -github.com/richardlehane/mscfb v1.0.6 h1:eN3bvvZCp00bs7Zf52bxNwAx5lJDBK1tCuH19qq5aC8= -github.com/richardlehane/mscfb v1.0.6/go.mod h1:pe0+IUIc0AHh0+teNzBlJCtSyZdFOGgV4ZK9bsoV+Jo= github.com/richardlehane/mscfb v1.0.7 h1:oeoiM0WE79vHwE8RpIYYvIAc8ajTH2mb6UZm55/+EB0= github.com/richardlehane/mscfb v1.0.7/go.mod h1:pe0+IUIc0AHh0+teNzBlJCtSyZdFOGgV4ZK9bsoV+Jo= github.com/richardlehane/msoleps v1.0.6 h1:9BvkpjvD+iUBalUY4esMwv6uBkfOip/Lzvd93jvR9gg= @@ -334,8 +334,6 @@ github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavM github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= github.com/xuri/efp v0.0.1 h1:fws5Rv3myXyYni8uwj2qKjVaRP30PdjeYe2Y6FDsCL8= github.com/xuri/efp v0.0.1/go.mod h1:ybY/Jr0T0GTCnYjKqmdwxyxn2BQf2RcQIIvex5QldPI= -github.com/xuri/excelize/v2 v2.10.1 h1:V62UlqopMqha3kOpnlHy2CcRVw1V8E63jFoWUmMzxN0= -github.com/xuri/excelize/v2 v2.10.1/go.mod h1:iG5tARpgaEeIhTqt3/fgXCGoBRt4hNXgCp3tfXKoOIc= github.com/xuri/excelize/v2 v2.11.0 h1:HxaEFl6sRN2+8J5a8HaKq+0M4FsjBGMnWWtjOCPSG88= github.com/xuri/excelize/v2 v2.11.0/go.mod h1:jxFLbzaIwGQ5ufFNvYfUOHqXhfPaNmP14KWfmNz2Uak= github.com/xuri/nfp v0.0.2-0.20250530014748-2ddeb826f9a9 h1:+C0TIdyyYmzadGaL/HBLbf3WdLgC29pgyhTjAT/0nuE= @@ -378,9 +376,8 @@ golang.org/x/crypto v0.53.0 h1:QZ4Muo8THX6CizN2vPPd5fBGHyogrdK9fG4wLPFUsto= golang.org/x/crypto v0.53.0/go.mod h1:DNLU434OwVakk9PzuwV8w62mAJpRJL3vsgcfp4Qnsio= golang.org/x/exp v0.0.0-20260218203240-3dfff04db8fa h1:Zt3DZoOFFYkKhDT3v7Lm9FDMEV06GpzjG2jrqW+QTE0= golang.org/x/exp v0.0.0-20260218203240-3dfff04db8fa/go.mod h1:K79w1Vqn7PoiZn+TkNpx3BUWUQksGO3JcVX6qIjytmA= -golang.org/x/image v0.27.0 h1:C8gA4oWU/tKkdCfYT6T2u4faJu3MeNS5O8UPWlPF61w= -golang.org/x/image v0.27.0/go.mod h1:xbdrClrAUway1MUTEZDq9mz/UpRwYAkFFNUslZtcB+g= golang.org/x/image v0.38.0 h1:5l+q+Y9JDC7mBOMjo4/aPhMDcxEptsX+Tt3GgRQRPuE= +golang.org/x/image v0.38.0/go.mod h1:/3f6vaXC+6CEanU4KJxbcUZyEePbyKbaLoDOe4ehFYY= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= diff --git a/http/http_test.go b/http/http_test.go index 7604baa..96c2d10 100644 --- a/http/http_test.go +++ b/http/http_test.go @@ -150,13 +150,14 @@ func TestHTTP(t *testing.T) { server := headerEchoServer(t) defer server.Close() + const hostOverride = "httpbin.example.com" resp, err := http.NewClient(). TraceToStdout(http.TraceAll). R(context.Background()). Header("Host", "example.test"). Get(server.URL) if err != nil { - t.Error(err) + t.Fatal(err) } headers := responseHeaders(t, resp) @@ -171,14 +172,14 @@ func TestHTTP(t *testing.T) { resp, err := http.NewClient().R(context.Background()).Header("Hello", "World").Get(server.URL) if err != nil { - t.Error(err) + t.Fatal(err) } headers := responseHeaders(t, resp) if headers["Hello"] != "World" { - t.Errorf("Expected response headers %s", headers) + t.Errorf("expected Hello header %q, got %v", "World", headers["Hello"]) } if v, ok := headers["Authorization"]; ok { - t.Errorf("Expecting blank authentication got %s", v) + t.Errorf("expected no Authorization header, got %v", v) } }) diff --git a/merge/merge.go b/merge/merge.go new file mode 100644 index 0000000..77e14bf --- /dev/null +++ b/merge/merge.go @@ -0,0 +1,459 @@ +// Package merge layers one configuration value over another, and deep-copies +// one, from the struct definitions themselves. +// +// It exists to delete a category of code: hand-written `if o.X != "" { s.X = o.X }` +// chains, hand-written emptiness tests, and hand-written field-by-field copies. +// Each of those has to be updated whenever a field is added, and each is a place +// a field can be silently forgotten. Structural merging cannot forget a field, +// so adding one to a spec needs no merge code at all. +// +// The default behaviour is: +// +// - scalars and strings — the override wins when it is set (a zero value means +// "unset", so an override can turn a bool on but never off); +// - slices — replaced wholesale when the override's is non-empty, because a +// list of tools means exactly those tools; +// - maps — merged key-wise, because a map is a set of independent settings; +// - structs, including those behind a pointer — merged field by field, so +// setting one sub-field does not erase its siblings. +// +// A single field can name an exception the structure cannot express, with a tag: +// +// Pre []string `merge:"append"` // the base's elements, then the override's +// Allow []string `merge:"append,unique"` // as append, with repeats dropped +// +// Tags are read on struct fields reached through structs and through pointers +// that are non-nil on both sides. A tag that cannot mean what it says — append on +// something that is not a list, unique over a non-comparable element type, a rule +// this package does not define — panics rather than being ignored, because a +// silently ignored tag reads exactly like an honoured one. +// +// Policy names the exceptions that belong to a whole type rather than to one +// field, including the types whose merge is a domain rule rather than a +// structural one and which therefore merge themselves. +package merge + +import ( + "fmt" + "reflect" + + "dario.cat/mergo" +) + +// reference identifies one pointer or map by the value it points at, so a walk +// can recognise a node it has already reached and stop rather than recurse for +// ever through a cycle. +type reference struct { + typ reflect.Type + ptr uintptr +} + +func referenceTo(v reflect.Value) reference { + return reference{typ: v.Type(), ptr: v.Pointer()} +} + +// tagName is the struct tag namespace for per-field merge rules. +const tagName = "merge" + +// Policy declares the exceptions to structural merging for one family of types. +// The zero Policy is valid and means "no exceptions". +type Policy struct { + // Replace lists types that are indivisible: when the override's value is set + // it replaces the base's wholesale instead of being merged into. A pointer + // type counts as set when it is non-nil — which is how an explicit + // *float64(0) or *bool(false) survives a merge that would otherwise read the + // pointed-at zero as "unset" and drop it. + // + // Name a type by its zero value: merge.Policy{Replace: []any{(*float64)(nil)}}. + Replace []any + + // Shared lists types that are referenced, not owned: Apply assigns them like + // Replace, and Clone copies the reference instead of walking it. Registries, + // catalogs and caches belong here — deep-copying one is both wasteful and + // wrong, because consumers rely on pointer identity. + Shared []any + + // Merger lists types that merge themselves. Each must declare the method + // `Merge(T) T` on the type named; Apply calls base.Merge(override) and takes + // the result instead of walking the type. It is where a merge rule that is a + // domain decision rather than a structural one lives: a list that accumulates + // across configuration layers instead of being replaced, or two fields that + // only mean anything when they move together. + // + // The method always runs — a nil slice or map destination is materialised + // first, so "the base had none" reaches Merge as an empty value rather than + // silently bypassing it. A pointer type therefore cannot be a Merger (a nil + // pointer has nothing to call the method on), and naming a type without that + // exact method panics rather than quietly reverting to structural merging. + Merger []any +} + +// With returns the union of two policies, so a nested type's policy can be +// composed into its container's rather than restated. +func (p Policy) With(other Policy) Policy { + return Policy{ + Replace: append(append([]any{}, p.Replace...), other.Replace...), + Shared: append(append([]any{}, p.Shared...), other.Shared...), + Merger: append(append([]any{}, p.Merger...), other.Merger...), + } +} + +// Apply returns base with override's set fields layered on top, per the package +// default and policy's exceptions. Neither argument is mutated, and the result +// shares no mutable memory with either — a merged spec can be edited without +// reaching back into the config it inherited from. +// +// T must be a struct, map or slice type; anything else is a caller error and +// panics, as does a mergo failure, which cannot occur for a well-formed T since +// both operands are the same concrete type by construction. +func Apply[T any](base, override T, policy Policy) T { + mergers := mergerSet(policy.Merger) + assign := typeSet(append(append([]any{}, policy.Replace...), policy.Shared...)) + out := clone(base, policy, mergers) + src := clone(override, policy, mergers) + + pre := accumulator{assign: assign, mergers: mergers, visited: map[[2]reference]bool{}} + pre.walk(reflect.ValueOf(&out).Elem(), reflect.ValueOf(&src).Elem(), fmt.Sprintf("%T", base)) + + opts := []func(*mergo.Config){mergo.WithOverride} + if len(assign) > 0 || len(mergers) > 0 { + opts = append(opts, mergo.WithTransformers(policyTransformer{assign: assign, mergers: mergers})) + } + if err := mergo.Merge(&out, src, opts...); err != nil { + panic(fmt.Sprintf("merge %T: %v", base, err)) + } + return out +} + +// accumulator carries the pre-pass over both layers: the types that own their +// merge rule, and the pairs of references already walked, so a cyclic value +// terminates instead of recursing until the stack is gone. +type accumulator struct { + assign map[reflect.Type]bool + mergers map[reflect.Type]bool + visited map[[2]reference]bool +} + +// walk rewrites each of the override's `append`-tagged slices to the +// concatenation of the base's and its own, ahead of the merge. The default rule — +// a non-empty override slice replaces the base's — then lands the accumulated +// value, so the tag needs no machinery of its own and cannot interact with the +// policy types, which own their rule and are not walked. +// +// It also settles the policy types held in a map, which the merge itself cannot: +// mergo reaches a map value through an unaddressable copy, so the transformer +// that implements Replace, Shared and Merger is handed a destination it cannot +// set, and the rule would silently do nothing. +func (a accumulator) walk(base, override reflect.Value, path string) { + if a.owns(base.Type()) { + return + } + switch base.Kind() { + case reflect.Pointer: + if !base.IsNil() && !override.IsNil() && a.enter(base, override) { + a.walk(base.Elem(), override.Elem(), path) + } + case reflect.Map: + a.walkMap(base, override, path) + case reflect.Struct: + for i := range base.NumField() { + field := base.Type().Field(i) + if !field.IsExported() { + continue + } + fieldPath := path + "." + field.Name + tag, tagged := field.Tag.Lookup(tagName) + if !tagged { + a.walk(base.Field(i), override.Field(i), fieldPath) + continue + } + if base.Field(i).Kind() != reflect.Slice { + panic(fmt.Sprintf("merge: %s is tagged %s:%q but is a %s, not a list", + fieldPath, tagName, tag, base.Field(i).Kind())) + } + if merged := concat(base.Field(i), override.Field(i), unique(tag, fieldPath), fieldPath); merged.IsValid() { + override.Field(i).Set(merged) + } + } + } +} + +// walkMap resolves each entry the override speaks about and writes the result to +// both layers, because which of the two the merge finally reads depends on the +// value's kind — a pointer entry keeps the destination's, a slice entry takes the +// source's — and agreeing on the answer makes that choice stop mattering. +func (a accumulator) walkMap(base, override reflect.Value, path string) { + if base.IsNil() || override.IsNil() || !a.enter(base, override) { + return + } + elem := base.Type().Elem() + // A map value is not addressable, so walking into one needs a copy that is. + settable := func(v reflect.Value) reflect.Value { + out := reflect.New(elem).Elem() + out.Set(v) + return out + } + for _, key := range override.MapKeys() { + entry := override.MapIndex(key) + if !entry.IsValid() { + continue + } + into := base.MapIndex(key) + + var resolved reflect.Value + switch { + case a.mergers[elem]: + // The rule always runs, so a key the base lacks reaches Merge as the + // empty value rather than bypassing it. + if !into.IsValid() { + into = cloneValue(reflect.New(elem).Elem(), nil, a.mergers) + } + resolved = into.MethodByName("Merge").Call([]reflect.Value{entry})[0] + case a.assign[elem]: + if resolved = entry; !isSet(entry) { + if !into.IsValid() { + continue + } + resolved = into + } + default: + if !into.IsValid() { + continue + } + baseEntry, overrideEntry := settable(into), settable(entry) + a.walk(baseEntry, overrideEntry, fmt.Sprintf("%s[%v]", path, key)) + base.SetMapIndex(key, baseEntry) + override.SetMapIndex(key, overrideEntry) + continue + } + base.SetMapIndex(key, resolved) + override.SetMapIndex(key, resolved) + } +} + +// owns reports whether a type declares its own merge rule, and is therefore +// neither walked for field tags nor merged structurally. +func (a accumulator) owns(typ reflect.Type) bool { return a.assign[typ] || a.mergers[typ] } + +// enter records a pair of references and reports whether this is the first time +// the walk has reached it. +func (a accumulator) enter(base, override reflect.Value) bool { + key := [2]reference{referenceTo(base), referenceTo(override)} + if a.visited[key] { + return false + } + a.visited[key] = true + return true +} + +// unique reports whether the tag asks for repeats to be dropped, and rejects any +// rule this package does not define. +func unique(tag, path string) bool { + switch tag { + case "append": + return false + case "append,unique": + return true + default: + panic(fmt.Sprintf("merge: %s declares %s:%q, want %q or %q", path, tagName, tag, "append", "append,unique")) + } +} + +// concat returns the base's elements followed by the override's, or the zero +// Value when neither layer contributed one and the field should be left as it is. +func concat(base, override reflect.Value, unique bool, path string) reflect.Value { + if base.Len() == 0 && override.Len() == 0 { + return reflect.Value{} + } + if unique && !base.Type().Elem().Comparable() { + panic(fmt.Sprintf("merge: %s asks for %s:%q but its element type %s is not comparable", + path, tagName, "append,unique", base.Type().Elem())) + } + out := reflect.MakeSlice(base.Type(), 0, base.Len()+override.Len()) + seen := map[any]bool{} + for _, layer := range []reflect.Value{base, override} { + for i := range layer.Len() { + if unique { + key := layer.Index(i).Interface() + if seen[key] { + continue + } + seen[key] = true + } + out = reflect.Append(out, layer.Index(i)) + } + } + return out +} + +// Clone returns a deep copy of v. Interfaces, functions and channels are copied +// by reference — they carry behaviour rather than configuration — as are the +// types named in policy.Shared. Unexported fields are copied as they stand. +func Clone[T any](v T, policy Policy) T { + return clone(v, policy, nil) +} + +// clone is Clone plus Apply's extra duty: materialise the nil slices and maps +// whose type declares its own Merge, so mergo — which skips its transformers for +// a nil destination — cannot bypass one. +func clone[T any](v T, policy Policy, materialize map[reflect.Type]bool) T { + shared := typeSet(policy.Shared) + if len(shared) == 0 { + shared = nil + } + return cloneValue(reflect.ValueOf(&v).Elem(), shared, materialize).Interface().(T) +} + +func cloneValue(v reflect.Value, shared, materialize map[reflect.Type]bool) reflect.Value { + return cloner{shared: shared, materialize: materialize, copies: map[reference]reflect.Value{}}.value(v) +} + +// cloner is one deep copy in progress. copies remembers the copy made for each +// pointer and map already reached, so a value that refers back to itself is +// copied once and pointed at again rather than followed for ever — and so two +// fields that referred to one value still do. +type cloner struct { + shared map[reflect.Type]bool + materialize map[reflect.Type]bool + copies map[reference]reflect.Value +} + +func (c cloner) value(v reflect.Value) reflect.Value { + if !v.IsValid() || c.shared[v.Type()] { + return v + } + switch v.Kind() { + case reflect.Pointer: + if v.IsNil() { + return v + } + if copied, ok := c.copies[referenceTo(v)]; ok { + return copied + } + out := reflect.New(v.Type().Elem()) + // Recorded before the target is copied: that is what a cycle reaching + // this pointer again finds instead of recursing. + c.copies[referenceTo(v)] = out + out.Elem().Set(c.value(v.Elem())) + return out + case reflect.Slice: + if v.IsNil() { + if !c.materialize[v.Type()] { + return v + } + return reflect.MakeSlice(v.Type(), 0, 0) + } + out := reflect.MakeSlice(v.Type(), v.Len(), v.Len()) + for i := range v.Len() { + out.Index(i).Set(c.value(v.Index(i))) + } + return out + case reflect.Map: + if v.IsNil() { + if !c.materialize[v.Type()] { + return v + } + return reflect.MakeMapWithSize(v.Type(), 0) + } + if copied, ok := c.copies[referenceTo(v)]; ok { + return copied + } + out := reflect.MakeMapWithSize(v.Type(), v.Len()) + c.copies[referenceTo(v)] = out + for iter := v.MapRange(); iter.Next(); { + out.SetMapIndex(c.value(iter.Key()), c.value(iter.Value())) + } + return out + case reflect.Array: + out := reflect.New(v.Type()).Elem() + for i := range v.Len() { + out.Index(i).Set(c.value(v.Index(i))) + } + return out + case reflect.Struct: + out := reflect.New(v.Type()).Elem() + out.Set(v) + for i := range v.NumField() { + if v.Type().Field(i).IsExported() { + out.Field(i).Set(c.value(v.Field(i))) + } + } + return out + default: + return v + } +} + +// policyTransformer is the mergo hook that short-circuits every policy type, +// keyed on the destination's type. +type policyTransformer struct { + assign map[reflect.Type]bool + mergers map[reflect.Type]bool +} + +func (t policyTransformer) Transformer(typ reflect.Type) func(dst, src reflect.Value) error { + switch { + case t.mergers[typ]: + return func(dst, src reflect.Value) error { + if dst.CanSet() { + dst.Set(dst.MethodByName("Merge").Call([]reflect.Value{src})[0]) + } + return nil + } + case t.assign[typ]: + return func(dst, src reflect.Value) error { + if isSet(src) && dst.CanSet() { + dst.Set(src) + } + return nil + } + default: + return nil + } +} + +// isSet reports whether an override value carries an instruction. A pointer is +// set when it is non-nil, so a pointer to a zero scalar still wins; every other +// kind is set when it is not the zero value. +func isSet(v reflect.Value) bool { + switch v.Kind() { + case reflect.Pointer, reflect.Interface, reflect.Map, reflect.Slice, reflect.Func, reflect.Chan: + return !v.IsNil() + default: + return !v.IsZero() + } +} + +// mergerSet validates that every Merger type can actually merge itself. The +// signature is checked here rather than at the merge, because a type whose method +// was renamed or re-typed would otherwise fall back to structural merging — the +// silent drift this package exists to remove. +func mergerSet(values []any) map[reflect.Type]bool { + set := typeSet(values) + for typ := range set { + if typ.Kind() == reflect.Pointer { + panic(fmt.Sprintf("merge: policy Merger type %s must not be a pointer — a nil pointer has no value to merge into", typ)) + } + method, ok := typ.MethodByName("Merge") + if !ok || method.Type.NumIn() != 2 || method.Type.In(1) != typ || + method.Type.NumOut() != 1 || method.Type.Out(0) != typ { + panic(fmt.Sprintf("merge: policy Merger type %s must declare a method Merge(%s) %s", typ, typ, typ)) + } + } + return set +} + +func typeSet(values []any) map[reflect.Type]bool { + if len(values) == 0 { + return nil + } + set := make(map[reflect.Type]bool, len(values)) + for _, value := range values { + typ := reflect.TypeOf(value) + if typ == nil { + panic("merge: policy type must be named by a typed zero value, e.g. (*float64)(nil)") + } + set[typ] = true + } + return set +} diff --git a/merge/merge_test.go b/merge/merge_test.go new file mode 100644 index 0000000..f80fbeb --- /dev/null +++ b/merge/merge_test.go @@ -0,0 +1,484 @@ +package merge_test + +import ( + "reflect" + "testing" + + "github.com/flanksource/commons/merge" +) + +// catalog stands in for a registry: a large shared structure reached by identity, +// which must be referenced rather than walked. +type catalog struct{ Entries []string } + +type limits struct { + Cost float64 + Retries *int +} + +type config struct { + Name string + Verbose bool + Tags []string + Settings map[string]string + Limits limits + Nested *limits + Rate *float64 + Catalog *catalog +} + +func floatPtr(v float64) *float64 { return &v } +func intPtr(v int) *int { return &v } + +func TestApply_Defaults(t *testing.T) { + base := config{ + Name: "base", + Tags: []string{"a", "b"}, + Settings: map[string]string{"one": "1", "two": "2"}, + Limits: limits{Cost: 5, Retries: intPtr(3)}, + } + override := config{ + Verbose: true, + Tags: []string{"c"}, + Settings: map[string]string{"two": "II"}, + Limits: limits{Cost: 9}, + } + + got := merge.Apply(base, override, merge.Policy{}) + + if got.Name != "base" { + t.Errorf("Name = %q, want the base's kept when the override is silent", got.Name) + } + if !got.Verbose { + t.Error("Verbose = false, want an override able to turn a flag on") + } + if !reflect.DeepEqual(got.Tags, []string{"c"}) { + t.Errorf("Tags = %v, want the override's list wholesale", got.Tags) + } + if !reflect.DeepEqual(got.Settings, map[string]string{"one": "1", "two": "II"}) { + t.Errorf("Settings = %v, want a key-wise merge", got.Settings) + } + // The override speaks about Cost only, so Retries — its sibling — survives. + if got.Limits.Cost != 9 || got.Limits.Retries == nil || *got.Limits.Retries != 3 { + t.Errorf("Limits = %+v, want Cost overridden and Retries kept", got.Limits) + } +} + +// A false or 0 in the override is indistinguishable from absent, so it must not +// erase the base. This is what makes a layered config predictable, and it is why +// an explicitly-zero value needs a pointer plus a Replace policy (below). +func TestApply_ZeroMeansUnset(t *testing.T) { + base := config{Name: "base", Verbose: true, Limits: limits{Cost: 5}} + + got := merge.Apply(base, config{}, merge.Policy{}) + + if got.Name != "base" || !got.Verbose || got.Limits.Cost != 5 { + t.Errorf("merged = %+v, want the base unchanged by an empty override", got) + } +} + +// The case a scalar pointer exists to express: an explicit zero. Without the +// Replace policy the engine would merge through the pointer, read the pointed-at +// 0 as unset, and silently keep the base — reintroducing the bug the pointer was +// added to prevent. +func TestApply_ReplaceKeepsExplicitZero(t *testing.T) { + policy := merge.Policy{Replace: []any{(*float64)(nil)}} + base := config{Rate: floatPtr(0.7)} + + if got := merge.Apply(base, config{Rate: floatPtr(0)}, policy); got.Rate == nil || *got.Rate != 0 { + t.Errorf("Rate = %v, want an explicit 0 to win", got.Rate) + } + if got := merge.Apply(base, config{}, policy); got.Rate == nil || *got.Rate != 0.7 { + t.Errorf("Rate = %v, want a nil override to leave the base alone", got.Rate) + } + if got := merge.Apply(config{}, config{Rate: floatPtr(0)}, policy); got.Rate == nil || *got.Rate != 0 { + t.Errorf("Rate = %v, want an explicit 0 to reach a base that had none", got.Rate) + } +} + +// A shared type is referenced, not owned: consumers rely on pointer identity, and +// walking one would copy an entire catalog per merge. +func TestApply_SharedIsReferencedNotCopied(t *testing.T) { + policy := merge.Policy{Shared: []any{(*catalog)(nil)}} + shared := &catalog{Entries: []string{"claude", "codex"}} + + got := merge.Apply(config{Catalog: shared}, config{}, policy) + if got.Catalog != shared { + t.Error("Catalog pointer changed, want the same instance shared through the merge") + } + + replacement := &catalog{Entries: []string{"gemini"}} + if got := merge.Apply(config{Catalog: shared}, config{Catalog: replacement}, policy); got.Catalog != replacement { + t.Error("Catalog = base's, want the override's instance") + } +} + +// Purity is the property that lets a merged value be edited freely: it must not +// reach back into the config it inherited from, on either side. +func TestApply_ResultSharesNoMemory(t *testing.T) { + base := config{Tags: []string{"a"}, Settings: map[string]string{"one": "1"}} + override := config{Nested: &limits{Cost: 5, Retries: intPtr(3)}} + + got := merge.Apply(base, override, merge.Policy{}) + got.Tags[0] = "mutated" + got.Settings["one"] = "mutated" + got.Nested.Cost = 99 + *got.Nested.Retries = 99 + + if base.Tags[0] != "a" || base.Settings["one"] != "1" { + t.Errorf("base mutated through the result: %+v", base) + } + if override.Nested.Cost != 5 || *override.Nested.Retries != 3 { + t.Errorf("override mutated through the result: %+v", override.Nested) + } +} + +// A nil destination pointer is the case mergo would otherwise satisfy by +// assigning the source pointer itself; Apply clones first so the result still +// owns its memory. +func TestApply_NilDestinationPointerIsNotAliased(t *testing.T) { + override := config{Nested: &limits{Cost: 5}} + + got := merge.Apply(config{}, override, merge.Policy{}) + if got.Nested == override.Nested { + t.Fatal("result aliases the override's pointer") + } + got.Nested.Cost = 99 + if override.Nested.Cost != 5 { + t.Errorf("override mutated through the result: %+v", override.Nested) + } +} + +func TestClone_IsDeepExceptShared(t *testing.T) { + shared := &catalog{Entries: []string{"claude"}} + original := config{ + Tags: []string{"a"}, + Settings: map[string]string{"one": "1"}, + Nested: &limits{Cost: 5, Retries: intPtr(3)}, + Catalog: shared, + } + + got := merge.Clone(original, merge.Policy{Shared: []any{(*catalog)(nil)}}) + if !reflect.DeepEqual(got, original) { + t.Fatalf("clone = %+v, want an equal value", got) + } + if got.Catalog != shared { + t.Error("Catalog was copied, want the shared instance referenced") + } + + got.Tags[0] = "mutated" + got.Settings["one"] = "mutated" + *got.Nested.Retries = 99 + if original.Tags[0] != "a" || original.Settings["one"] != "1" || *original.Nested.Retries != 3 { + t.Errorf("original mutated through the clone: %+v", original) + } +} + +// tagList accumulates across layers and drops repeats. That is a statement about +// what a list of tags means, which no structural walk could infer — the whole +// reason a type gets to merge itself. +type tagList []string + +func (base tagList) Merge(override tagList) tagList { + seen := make(map[string]bool, len(base)+len(override)) + var out tagList + for _, tag := range append(append(tagList{}, base...), override...) { + if seen[tag] { + continue + } + seen[tag] = true + out = append(out, tag) + } + return out +} + +type layered struct { + Name string + Tags tagList +} + +func TestApply_MergerOwnsItsRule(t *testing.T) { + policy := merge.Policy{Merger: []any{tagList(nil)}} + base := layered{Name: "base", Tags: tagList{"a", "b"}} + + got := merge.Apply(base, layered{Tags: tagList{"b", "c"}}, policy) + + if !reflect.DeepEqual(got.Tags, tagList{"a", "b", "c"}) { + t.Errorf("Tags = %v, want the layers accumulated and deduped, not replaced", got.Tags) + } + if !reflect.DeepEqual(base.Tags, tagList{"a", "b"}) { + t.Errorf("base mutated through the merge: %v", base.Tags) + } +} + +// mergo skips its transformers for a nil destination, which would let "the base +// had no tags" quietly bypass the rule and assign the override wholesale — repeats +// and all. Apply materialises the nil first, so Merge always gets to speak. +func TestApply_MergerRunsAgainstANilBase(t *testing.T) { + policy := merge.Policy{Merger: []any{tagList(nil)}} + + got := merge.Apply(layered{Name: "base"}, layered{Tags: tagList{"c", "c"}}, policy) + + if !reflect.DeepEqual(got.Tags, tagList{"c"}) { + t.Errorf("Tags = %v, want the type's dedupe applied even with nothing to merge into", got.Tags) + } +} + +func TestPolicy_MergerWithoutTheMethodPanics(t *testing.T) { + for name, policy := range map[string]merge.Policy{ + "no Merge method": {Merger: []any{catalog{}}}, + "pointer type": {Merger: []any{(*tagList)(nil)}}, + } { + t.Run(name, func(t *testing.T) { + defer func() { + if recover() == nil { + t.Error("no panic, want the policy rejected instead of silently merging structurally") + } + }() + merge.Apply(layered{}, layered{}, policy) + }) + } +} + +// A configuration layer's hooks add to the layers beneath it rather than +// replacing them, and its allow-list does the same without repeating itself. +// Both are statements about what those particular fields mean, so they are made +// on the fields — the alternative is a named slice type and a hand-written Merge +// method per list, which is the plumbing this package exists to delete. +type policyLayer struct { + Pre []string `merge:"append"` + Allow []string `merge:"append,unique"` + Files []string + Nested *policyLayer +} + +func TestApply_AppendTagAccumulatesAcrossLayers(t *testing.T) { + base := policyLayer{Pre: []string{"lint"}, Allow: []string{"a", "b"}, Files: []string{"base.md"}} + override := policyLayer{Pre: []string{"build"}, Allow: []string{"b", "c"}, Files: []string{"over.md"}} + + got := merge.Apply(base, override, merge.Policy{}) + + if !reflect.DeepEqual(got.Pre, []string{"lint", "build"}) { + t.Errorf("Pre = %v, want the base's steps and then the override's", got.Pre) + } + if !reflect.DeepEqual(got.Allow, []string{"a", "b", "c"}) { + t.Errorf("Allow = %v, want the layers accumulated with repeats dropped", got.Allow) + } + if !reflect.DeepEqual(got.Files, []string{"over.md"}) { + t.Errorf("Files = %v, want an untagged slice still replaced wholesale", got.Files) + } + if !reflect.DeepEqual(base.Pre, []string{"lint"}) { + t.Errorf("base mutated through the merge: %v", base.Pre) + } +} + +func TestApply_AppendTagIsReadThroughAPointer(t *testing.T) { + base := policyLayer{Nested: &policyLayer{Pre: []string{"lint"}}} + override := policyLayer{Nested: &policyLayer{Pre: []string{"build"}}} + + got := merge.Apply(base, override, merge.Policy{}) + + if !reflect.DeepEqual(got.Nested.Pre, []string{"lint", "build"}) { + t.Errorf("Nested.Pre = %v, want the tag honoured behind a pointer too", got.Nested.Pre) + } +} + +// Accumulating with nothing to accumulate is still the identity in both +// directions: an empty layer neither erases the one beneath it nor is erased by it. +func TestApply_AppendTagWithOneSideEmpty(t *testing.T) { + if got := merge.Apply(policyLayer{Pre: []string{"lint"}}, policyLayer{}, merge.Policy{}); !reflect.DeepEqual(got.Pre, []string{"lint"}) { + t.Errorf("Pre = %v, want an empty override to leave the base alone", got.Pre) + } + if got := merge.Apply(policyLayer{}, policyLayer{Pre: []string{"build"}}, merge.Policy{}); !reflect.DeepEqual(got.Pre, []string{"build"}) { + t.Errorf("Pre = %v, want the override to reach a base that had none", got.Pre) + } +} + +type nonComparableElements struct { + Groups [][]string `merge:"append,unique"` +} + +type appendOnAScalar struct { + Name string `merge:"append"` +} + +type unknownRule struct { + Pre []string `merge:"accumulate"` +} + +// A tag that cannot mean what it says must fail at the merge rather than +// degrading to plain append or to no rule at all — a silently ignored tag reads +// exactly like an honoured one. +func TestApply_UnsatisfiableTagPanics(t *testing.T) { + for name, apply := range map[string]func(){ + "unique over a non-comparable element type": func() { + merge.Apply( + nonComparableElements{Groups: [][]string{{"a"}}}, + nonComparableElements{Groups: [][]string{{"b"}}}, + merge.Policy{}) + }, + "append on something that is not a list": func() { + merge.Apply(appendOnAScalar{Name: "base"}, appendOnAScalar{Name: "override"}, merge.Policy{}) + }, + "a rule the package does not define": func() { + merge.Apply(unknownRule{}, unknownRule{}, merge.Policy{}) + }, + } { + t.Run(name, func(t *testing.T) { + defer func() { + if recover() == nil { + t.Error("no panic, want the tag rejected instead of silently ignored") + } + }() + apply() + }) + } +} + +func TestPolicy_With_IsTheUnion(t *testing.T) { + got := merge.Policy{Replace: []any{(*float64)(nil)}, Shared: []any{(*catalog)(nil)}}. + With(merge.Policy{Replace: []any{(*int)(nil)}, Merger: []any{tagList(nil)}}) + + if len(got.Replace) != 2 || len(got.Shared) != 1 || len(got.Merger) != 1 { + t.Errorf("union = %+v, want both Replace entries, the Shared entry and the Merger entry", got) + } +} + +// A map is merged key-wise, so its values are merged too — and a policy is a +// statement about a type, not about where the type is reached from. The merge +// engine hands a transformer an unaddressable copy of a map value, so a rule +// applied only there would silently do nothing to an entry both layers hold. +type registry struct { + Rates map[string]*float64 + Tags map[string]tagList + Catalogs map[string]*catalog +} + +func TestApply_ReplaceAppliesToAMapEntry(t *testing.T) { + policy := merge.Policy{Replace: []any{(*float64)(nil)}} + base := registry{Rates: map[string]*float64{"a": floatPtr(0.7), "b": floatPtr(0.1)}} + + got := merge.Apply(base, registry{Rates: map[string]*float64{"a": floatPtr(0)}}, policy) + + if got.Rates["a"] == nil || *got.Rates["a"] != 0 { + t.Errorf("Rates[a] = %v, want an explicit 0 to win inside a map too", got.Rates["a"]) + } + if got.Rates["b"] == nil || *got.Rates["b"] != 0.1 { + t.Errorf("Rates[b] = %v, want a key the override is silent about kept", got.Rates["b"]) + } +} + +func TestApply_MergerAppliesToAMapEntry(t *testing.T) { + policy := merge.Policy{Merger: []any{tagList(nil)}} + base := registry{Tags: map[string]tagList{"a": {"x", "y"}}} + + got := merge.Apply(base, registry{Tags: map[string]tagList{"a": {"y", "z"}, "b": {"q", "q"}}}, policy) + + if !reflect.DeepEqual(got.Tags["a"], tagList{"x", "y", "z"}) { + t.Errorf("Tags[a] = %v, want the entry accumulated and deduped, not replaced", got.Tags["a"]) + } + // The rule always runs: a key the base lacks reaches Merge as the empty value + // rather than being assigned wholesale, repeats and all. + if !reflect.DeepEqual(got.Tags["b"], tagList{"q"}) { + t.Errorf("Tags[b] = %v, want the type's dedupe applied to a new key", got.Tags["b"]) + } + if !reflect.DeepEqual(base.Tags["a"], tagList{"x", "y"}) { + t.Errorf("base mutated through the merge: %v", base.Tags["a"]) + } +} + +func TestApply_SharedMapEntryIsReferencedNotCopied(t *testing.T) { + policy := merge.Policy{Shared: []any{(*catalog)(nil)}} + shared := &catalog{Entries: []string{"claude"}} + replacement := &catalog{Entries: []string{"gemini"}} + + got := merge.Apply( + registry{Catalogs: map[string]*catalog{"a": shared, "b": shared}}, + registry{Catalogs: map[string]*catalog{"b": replacement}}, + policy) + + if got.Catalogs["a"] != shared { + t.Error("Catalogs[a] was copied, want the shared instance referenced") + } + if got.Catalogs["b"] != replacement { + t.Error("Catalogs[b] = base's, want the override's instance") + } +} + +// A value that refers back to itself must be copied once and pointed at again. +// Following it instead ends the process in a stack overflow, which no caller can +// recover from — and a config graph is not guaranteed to be a tree. +func TestClone_CyclicValueTerminates(t *testing.T) { + original := &node{Name: "a"} + original.Next = original + + got := merge.Clone(original, merge.Policy{}) + + if got == original { + t.Fatal("clone aliases the original") + } + if got.Next != got { + t.Errorf("Next = %p, want the self-reference preserved as one (%p)", got.Next, got) + } +} + +func TestClone_CyclicMapTerminates(t *testing.T) { + original := selfMap{} + original["self"] = original + + if got := merge.Clone(original, merge.Policy{}); got["self"] == nil { + t.Error("self-referencing key lost") + } +} + +func TestApply_CyclicValueTerminates(t *testing.T) { + cycle := func(name string) graph { + n := &node{Name: name} + n.Next = n + return graph{Root: n} + } + + got := merge.Apply(cycle("base"), cycle("override"), merge.Policy{}) + + if got.Root.Name != "override" { + t.Errorf("Root.Name = %q, want the override's", got.Root.Name) + } + if got.Root.Next != got.Root { + t.Error("cycle not preserved through the merge") + } +} + +// Two fields that referred to one value still do: the clone owns its memory, but +// it is the same shape as what it copied. +func TestClone_PreservesAliasing(t *testing.T) { + shared := &limits{Cost: 5} + + got := merge.Clone(struct{ A, B *limits }{A: shared, B: shared}, merge.Policy{}) + + if got.A == shared { + t.Fatal("clone aliases the original") + } + if got.A != got.B { + t.Error("two references to one value became two values") + } +} + +type node struct { + Name string + Next *node +} + +type graph struct{ Root *node } + +type selfMap map[string]selfMap + +// Naming a policy type by an untyped nil carries no type information, so it can +// only silently do nothing — fail loud instead. +func TestPolicy_UntypedNilPanics(t *testing.T) { + defer func() { + if recover() == nil { + t.Error("no panic for an untyped nil policy entry") + } + }() + merge.Apply(config{}, config{}, merge.Policy{Replace: []any{nil}}) +}