refactor: Reducing complexity of the ketchupsToNotify func

Signed-off-by: Vincent Boutour <[email protected]>
ViBiOh · May 29, 2021 · a71e573 · a71e573
1 parent a44458b
commit a71e573
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 45 deletions.
diff --git a/pkg/model/ketchup.go b/pkg/model/ketchup.go
@@ -62,12 +62,12 @@ func NewKetchup(pattern, version string, frequency KetchupFrequency, repo Reposi
 	}
 }
 
-// KetchupByRepositoryID sort ketchup by repository ID
-type KetchupByRepositoryID []Ketchup
+// KetchupByRepositoryIDAndPattern sort ketchup by repository ID
+type KetchupByRepositoryIDAndPattern []Ketchup
 
-func (a KetchupByRepositoryID) Len() int      { return len(a) }
-func (a KetchupByRepositoryID) Swap(i, j int) { a[i], a[j] = a[j], a[i] }
-func (a KetchupByRepositoryID) Less(i, j int) bool {
+func (a KetchupByRepositoryIDAndPattern) Len() int      { return len(a) }
+func (a KetchupByRepositoryIDAndPattern) Swap(i, j int) { a[i], a[j] = a[j], a[i] }
+func (a KetchupByRepositoryIDAndPattern) Less(i, j int) bool {
 	if a[i].Repository.ID == a[j].Repository.ID {
 		return a[i].Pattern < a[j].Pattern
 	}
@@ -123,12 +123,12 @@ func NewRelease(repository Repository, pattern string, version semver.Version) R
 	}
 }
 
-// ReleaseByRepositoryID sort release by repository ID
-type ReleaseByRepositoryID []Release
+// ReleaseByRepositoryIDAndPattern sort release by repository ID
+type ReleaseByRepositoryIDAndPattern []Release
 
-func (a ReleaseByRepositoryID) Len() int      { return len(a) }
-func (a ReleaseByRepositoryID) Swap(i, j int) { a[i], a[j] = a[j], a[i] }
-func (a ReleaseByRepositoryID) Less(i, j int) bool {
+func (a ReleaseByRepositoryIDAndPattern) Len() int      { return len(a) }
+func (a ReleaseByRepositoryIDAndPattern) Swap(i, j int) { a[i], a[j] = a[j], a[i] }
+func (a ReleaseByRepositoryIDAndPattern) Less(i, j int) bool {
 	if a[i].Repository.ID == a[j].Repository.ID {
 		return a[i].Pattern < a[j].Pattern
 	}

diff --git a/pkg/model/ketchup_test.go b/pkg/model/ketchup_test.go
@@ -99,7 +99,7 @@ func TestKetchupByRepositoryID(t *testing.T) {
 
 	for _, tc := range cases {
 		t.Run(tc.intention, func(t *testing.T) {
-			sort.Sort(KetchupByRepositoryID(tc.args.array))
+			sort.Sort(KetchupByRepositoryIDAndPattern(tc.args.array))
 			if got := tc.args.array; !reflect.DeepEqual(got, tc.want) {
 				t.Errorf("KetchupByRepositoryID() = %+v, want %+v", got, tc.want)
 			}
@@ -217,7 +217,7 @@ func TestReleaseByRepositoryID(t *testing.T) {
 
 	for _, tc := range cases {
 		t.Run(tc.intention, func(t *testing.T) {
-			sort.Sort(ReleaseByRepositoryID(tc.args.array))
+			sort.Sort(ReleaseByRepositoryIDAndPattern(tc.args.array))
 			if got := tc.args.array; !reflect.DeepEqual(got, tc.want) {
 				t.Errorf("ReleaseByRepositoryID() = %+v, want %+v", got, tc.want)
 			}

diff --git a/pkg/notifier/notifier.go b/pkg/notifier/notifier.go
@@ -81,7 +81,7 @@ func (a app) Notify(ctx context.Context) error {
 		return fmt.Errorf("unable to get new releases: %w", err)
 	}
 
-	sort.Sort(model.ReleaseByRepositoryID(newReleases))
+	sort.Sort(model.ReleaseByRepositoryIDAndPattern(newReleases))
 	if err := a.updateRepositories(userCtx, newReleases); err != nil {
 		return fmt.Errorf("unable to update repositories: %w", err)
 	}
@@ -147,10 +147,29 @@ func (a app) getKetchupToNotify(ctx context.Context, releases []model.Release) (
 		return nil, fmt.Errorf("unable to get ketchups for repositories: %w", err)
 	}
 
-	userToNotify := make(map[model.User][]model.Release)
+	userToNotify := syncReleasesByUser(releases, ketchups)
+	logger.Info("%d daily ketchups to notify", len(ketchups))
+
+	if a.clock.Now().Weekday() == time.Monday {
+		weeklyKetchups, err := a.ketchupService.ListOutdatedByFrequency(ctx, model.Weekly)
+		if err != nil {
+			return nil, fmt.Errorf("unable to get weekly ketchups: %w", err)
+		}
+
+		logger.Info("%d weekly ketchups to notify", len(weeklyKetchups))
+		addWeeklyKetchups(weeklyKetchups, userToNotify)
+	}
+
+	logger.Info("%d users to notify", len(userToNotify))
+
+	return userToNotify, nil
+}
+
+func syncReleasesByUser(releases []model.Release, ketchups []model.Ketchup) map[model.User][]model.Release {
+	usersToNotify := make(map[model.User][]model.Release)
 
-	sort.Sort(model.ReleaseByRepositoryID(releases))
-	sort.Sort(model.KetchupByRepositoryID(ketchups))
+	sort.Sort(model.ReleaseByRepositoryIDAndPattern(releases))
+	sort.Sort(model.KetchupByRepositoryIDAndPattern(ketchups))
 
 	index := 0
 	size := len(ketchups)
@@ -160,54 +179,44 @@ func (a app) getKetchupToNotify(ctx context.Context, releases []model.Release) (
 			current := ketchups[index]
 
 			if release.Repository.ID < current.Repository.ID || (release.Repository.ID == current.Repository.ID && release.Pattern < current.Pattern) {
-				break // release is out of sync, we need to advance
+				break // release is out of sync, we need to go foward release
 			}
 
 			index++
+
 			if release.Repository.ID != current.Repository.ID || release.Pattern != current.Pattern {
-				continue // ketchup is not sync with release
+				continue // ketchup is not sync with release, we need for go forward ketchup
 			}
 
 			if current.Version != release.Version.Name {
-				if userToNotify[current.User] != nil {
-					userToNotify[current.User] = append(userToNotify[current.User], release)
+				if usersToNotify[current.User] != nil {
+					usersToNotify[current.User] = append(usersToNotify[current.User], release)
 				} else {
-					userToNotify[current.User] = []model.Release{release}
+					usersToNotify[current.User] = []model.Release{release}
 				}
 			}
 		}
 	}
 
-	logger.Info("%d daily ketchups to notify", len(ketchups))
+	return usersToNotify
+}
 
-	if a.clock.Now().Weekday() == time.Monday {
-		weeklyKetchups, err := a.ketchupService.ListOutdatedByFrequency(ctx, model.Weekly)
+func addWeeklyKetchups(ketchups []model.Ketchup, usersToNotify map[model.User][]model.Release) {
+	for _, ketchup := range ketchups {
+		ketchupVersion, err := semver.Parse(ketchup.Version)
 		if err != nil {
-			return nil, fmt.Errorf("unable to get weekly ketchups: %w", err)
+			logger.WithField("version", ketchup.Version).Error("unable to parse version of ketchup: %s", err)
+			continue
 		}
 
-		for _, ketchup := range weeklyKetchups {
-			ketchupVersion, err := semver.Parse(ketchup.Version)
-			if err != nil {
-				logger.WithField("version", ketchup.Version).Error("unable to parse version of ketchup: %s", err)
-				continue
-			}
-
-			release := model.NewRelease(ketchup.Repository, ketchup.Pattern, ketchupVersion)
+		release := model.NewRelease(ketchup.Repository, ketchup.Pattern, ketchupVersion)
 
-			if userToNotify[ketchup.User] != nil {
-				userToNotify[ketchup.User] = append(userToNotify[ketchup.User], release)
-			} else {
-				userToNotify[ketchup.User] = []model.Release{release}
-			}
+		if usersToNotify[ketchup.User] != nil {
+			usersToNotify[ketchup.User] = append(usersToNotify[ketchup.User], release)
+		} else {
+			usersToNotify[ketchup.User] = []model.Release{release}
 		}
-
-		logger.Info("%d weekly ketchups to notify", len(weeklyKetchups))
 	}
-
-	logger.Info("%d ketchups for %d users to notify", len(ketchups), len(userToNotify))
-
-	return userToNotify, nil
 }
 
 func (a app) sendNotification(ctx context.Context, ketchupToNotify map[model.User][]model.Release) error {

diff --git a/pkg/notifier/releases_test.go b/pkg/notifier/releases_test.go
@@ -217,7 +217,7 @@ func TestGetNewHelmReleases(t *testing.T) {
 
 			failed := false
 
-			sort.Sort(model.ReleaseByRepositoryID(got))
+			sort.Sort(model.ReleaseByRepositoryIDAndPattern(got))
 
 			if tc.wantErr == nil && gotErr != nil {
 				failed = true