fix: Ensure no duplicates in Global configuration

pkg/config/config.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"log/slog"
+	"strings"
 
 	"github.com/hashicorp/go-multierror"
 	"github.com/jippi/scm-engine/pkg/scm"
@@ -131,22 +132,100 @@ func (c *Config) LoadIncludes(ctx context.Context, client scm.Client) error {
 }
 
 // Merge merges the other config into the current config
-func (c *Config) Merge(other *Config) {
+func (c *Config) Merge(other *Config) *Config {
+	cfg := &Config{}
+
 	if other == nil {
-		return
+		return &Config{
+			DryRun:             c.DryRun,
+			IgnoreActivityFrom: c.IgnoreActivityFrom,
+			Actions:            c.Actions,
+			Labels:             c.Labels,
+			Includes:           c.Includes,
+		}
+	}
+
+	cfg.DryRun = other.DryRun
+
+	cfg.IgnoreActivityFrom.IsBot = other.IgnoreActivityFrom.IsBot
+
+	if c.IgnoreActivityFrom.Usernames != nil || other.IgnoreActivityFrom.Usernames != nil {
+		cfg.IgnoreActivityFrom.Usernames = scm.MergeSlices(c.IgnoreActivityFrom.Usernames, other.IgnoreActivityFrom.Usernames, func(username string) string {
+			return username
+		})
 	}
 
-	c.DryRun = other.DryRun
+	if c.IgnoreActivityFrom.Emails != nil || other.IgnoreActivityFrom.Emails != nil {
+		cfg.IgnoreActivityFrom.Emails = scm.MergeSlices(c.IgnoreActivityFrom.Emails, other.IgnoreActivityFrom.Emails, func(email string) string {
+			return email
+		})
+	}
 
-	c.IgnoreActivityFrom.IsBot = other.IgnoreActivityFrom.IsBot
-	c.IgnoreActivityFrom.Usernames = append(c.IgnoreActivityFrom.Usernames, other.IgnoreActivityFrom.Usernames...)
-	c.IgnoreActivityFrom.Emails = append(c.IgnoreActivityFrom.Emails, other.IgnoreActivityFrom.Emails...)
+	if c.Actions != nil || other.Actions != nil {
+		cfg.Actions = scm.MergeSlices(c.Actions, other.Actions, func(action Action) string {
+			return action.Name
+		})
+	}
+
+	if c.Labels != nil || other.Labels != nil {
+		cfg.Labels = scm.MergeSlices(c.Labels, other.Labels, func(label *Label) string {
+			return label.Name
+		})
+	}
+
+	// Merge includes, but skip adding duplicate files under a project/ref
+	if c.Includes != nil || other.Includes != nil {
+		includes := map[string]map[string]*bool{}
+
+		for _, include := range c.Includes {
+			for _, file := range include.Files {
+				if _, ok := includes[key(include.Project, include.Ref)]; !ok {
+					includes[key(include.Project, include.Ref)] = map[string]*bool{}
+				}
+
+				includes[key(include.Project, include.Ref)][file] = nil
+			}
+		}
 
-	c.Actions = append(c.Actions, other.Actions...)
-	c.Labels = append(c.Labels, other.Labels...)
+		for _, include := range other.Includes {
+			for _, file := range include.Files {
+				if _, ok := includes[key(include.Project, include.Ref)]; !ok {
+					includes[key(include.Project, include.Ref)] = map[string]*bool{}
+				}
 
-	// don't have to worry about duplication here, it is handled when loading the includes
-	c.Includes = append(c.Includes, other.Includes...)
+				includes[key(include.Project, include.Ref)][file] = nil
+			}
+		}
+
+		cfg.Includes = make([]Include, 0, len(includes))
+
+		for key, fileMap := range includes {
+			keyParts := strings.Split(key, ":")
+			project := keyParts[0]
+			ref := keyParts[1]
+
+			files := make([]string, 0, len(fileMap))
+			for file := range fileMap {
+				files = append(files, file)
+			}
+
+			cfg.Includes = append(cfg.Includes, Include{
+				Project: project,
+				Ref:     scm.Ptr(ref),
+				Files:   files,
+			})
+		}
+	}
+
+	return cfg
+}
+
+func key(project string, ref *string) string {
+	strRef := ""
+
+	if ref != nil {
+		strRef = *ref
+	}
 
-	return
+	return fmt.Sprintf("%s:%s", project, strRef)
 }

pkg/config/config_test.go

@@ -44,42 +44,42 @@ func TestConfig_Merge(t *testing.T) {
 			cfg: &config.Config{
 				IgnoreActivityFrom: config.IgnoreActivityFrom{
 					IsBot:     false,
-					Usernames: []string{"user1"},
+					Usernames: []string{"user1", "user2"},
 					Emails:    []string{"[email protected]"},
 				},
 			},
 			other: &config.Config{
 				IgnoreActivityFrom: config.IgnoreActivityFrom{
 					IsBot:     true,
-					Usernames: []string{"user3"},
+					Usernames: []string{"user3", "user2"},
 					Emails:    []string{"[email protected]"},
 				},
 			},
 			want: &config.Config{
 				IgnoreActivityFrom: config.IgnoreActivityFrom{
 					IsBot:     true,
-					Usernames: []string{"user1", "user3"},
+					Usernames: []string{"user1", "user2", "user3"},
 					Emails:    []string{"[email protected]", "[email protected]"},
 				},
 			},
 		},
 		{
 			name: "merge actions",
 			cfg: &config.Config{
-				Actions: []config.Action{{Name: "action1"}},
+				Actions: []config.Action{{Name: "action1"}, {Name: "action2"}},
 			},
 			other: &config.Config{
-				Actions: []config.Action{{Name: "action2"}},
+				Actions: []config.Action{{Name: "action3"}, {Name: "action2"}},
 			},
-			want: &config.Config{Actions: []config.Action{{Name: "action1"}, {Name: "action2"}}},
+			want: &config.Config{Actions: []config.Action{{Name: "action1"}, {Name: "action2"}, {Name: "action3"}}},
 		},
 		{
 			name: "merge labels",
-			cfg:  &config.Config{Labels: config.Labels{{Name: "label1"}}},
+			cfg:  &config.Config{Labels: config.Labels{{Name: "label1"}, {Name: "label2"}}},
 			other: &config.Config{
-				Labels: config.Labels{{Name: "label2"}},
+				Labels: config.Labels{{Name: "label3"}, {Name: "label2"}},
 			},
-			want: &config.Config{Labels: config.Labels{{Name: "label1"}, {Name: "label2"}}},
+			want: &config.Config{Labels: config.Labels{{Name: "label1"}, {Name: "label2"}, {Name: "label3"}}},
 		},
 		{
 			name: "merge includes",
@@ -89,8 +89,7 @@ func TestConfig_Merge(t *testing.T) {
 			},
 			want: &config.Config{
 				Includes: []config.Include{
-					{Project: "project1", Ref: scm.Ptr("ref1"), Files: []string{"file1"}},
-					{Project: "project1", Ref: scm.Ptr("ref1"), Files: []string{"file2"}},
+					{Project: "project1", Ref: scm.Ptr("ref1"), Files: []string{"file1", "file2"}},
 				},
 			},
 		},
@@ -100,8 +99,8 @@ func TestConfig_Merge(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			tt.cfg.Merge(tt.other)
-			require.Equal(t, tt.want, tt.cfg)
+			got := tt.cfg.Merge(tt.other)
+			require.Equal(t, tt.want, got)
 		})
 	}
 }

pkg/scm/helpers.go

@@ -203,3 +203,24 @@ func buildPatternRegex(pattern string) (*regexp.Regexp, error) {
 
 	return regexp.Compile(regexString.String())
 }
+
+func MergeSlices[T any](aSlice []T, bSlice []T, uniqFunc func(T) string) []T {
+	uniques := map[string]bool{}
+	result := []T{}
+
+	for _, item := range aSlice {
+		uniques[uniqFunc(item)] = true
+
+		result = append(result, item)
+	}
+
+	for _, item := range bSlice {
+		if _, ok := uniques[uniqFunc(item)]; !ok {
+			uniques[uniqFunc(item)] = true
+
+			result = append(result, item)
+		}
+	}
+
+	return result
+}