refactor: Reudcing cyclomatic complexity for getNewReleases

Signed-off-by: Vincent Boutour <[email protected]>
ViBiOh · Feb 17, 2021 · 76ca1e8 · 76ca1e8
1 parent ad203ea
commit 76ca1e8
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 34 deletions.
diff --git a/pkg/scheduler/scheduler.go b/pkg/scheduler/scheduler.go
@@ -90,6 +90,11 @@ func (a app) ketchupNotify(_ time.Time) error {
 		return fmt.Errorf("unable to get new releases: %w", err)
 	}
 
+	sort.Sort(model.ReleaseByRepositoryID(newReleases))
+	if err := a.updateRepositories(ctx, newReleases); err != nil {
+		return fmt.Errorf("unable to update repositories: %w", err)
+	}
+
 	ketchupsToNotify, err := a.getKetchupToNotify(ctx, newReleases)
 	if err != nil {
 		return fmt.Errorf("unable to get ketchup to notify: %w", err)
@@ -115,20 +120,7 @@ func (a app) getNewReleases(ctx context.Context) ([]model.Release, error) {
 
 		for _, repo := range repositories {
 			count++
-
-			releases := a.checkRepositoryVersion(repo)
-			if len(releases) == 0 {
-				continue
-			}
-
-			newReleases = append(newReleases, releases...)
-			for _, release := range releases {
-				repo.Versions[release.Pattern] = release.Version.Name
-			}
-
-			if err := a.repositoryService.Update(ctx, repo); err != nil {
-				return nil, fmt.Errorf("unable to update repo `%s`: %s", repo.Name, err)
-			}
+			newReleases = append(newReleases, a.getNewRepositoryReleases(repo)...)
 		}
 
 		if uint64(page*pageSize) < totalCount {
@@ -140,7 +132,7 @@ func (a app) getNewReleases(ctx context.Context) ([]model.Release, error) {
 	}
 }
 
-func (a app) checkRepositoryVersion(repo model.Repository) []model.Release {
+func (a app) getNewRepositoryReleases(repo model.Repository) []model.Release {
 	versions, err := a.repositoryService.LatestVersions(repo)
 	if err != nil {
 		logger.Error("unable to get latest versions of %s: %s", repo.Name, err)
@@ -178,6 +170,32 @@ func (a app) checkRepositoryVersion(repo model.Repository) []model.Release {
 	return releases
 }
 
+func (a app) updateRepositories(ctx context.Context, releases []model.Release) error {
+	if len(releases) == 0 {
+		return nil
+	}
+
+	repo := releases[0].Repository
+
+	for _, release := range releases {
+		if release.Repository.ID != repo.ID {
+			if err := a.repositoryService.Update(ctx, repo); err != nil {
+				return fmt.Errorf("unable to update repository `%s`: %s", repo.Name, err)
+			}
+
+			repo = release.Repository
+		}
+
+		repo.Versions[release.Pattern] = release.Version.Name
+	}
+
+	if err := a.repositoryService.Update(ctx, repo); err != nil {
+		return fmt.Errorf("unable to update repository `%s`: %s", repo.Name, err)
+	}
+
+	return nil
+}
+
 func (a app) getKetchupToNotify(ctx context.Context, releases []model.Release) (map[model.User][]model.Release, error) {
 	repositories := make([]model.Repository, len(releases))
 	for index, release := range releases {

diff --git a/pkg/scheduler/scheduler_test.go b/pkg/scheduler/scheduler_test.go
@@ -113,21 +113,6 @@ func TestGetNewReleases(t *testing.T) {
 			nil,
 			nil,
 		},
-		{
-			"update error",
-			app{
-				repositoryService: repositorytest.New().SetList([]model.Repository{
-					model.NewRepository(1, model.Github, repositoryName).AddVersion(model.DefaultPattern, repositoryVersion),
-				}, 0, nil).SetLatestVersions(map[string]semver.Version{
-					model.DefaultPattern: safeParse("1.1.0"),
-				}, nil).SetUpdate(errors.New("failed")),
-			},
-			args{
-				ctx: context.Background(),
-			},
-			nil,
-			errors.New("unable to update repo `vibioh/ketchup`"),
-		},
 		{
 			"success",
 			app{
@@ -141,7 +126,7 @@ func TestGetNewReleases(t *testing.T) {
 				ctx: context.Background(),
 			},
 			[]model.Release{model.NewRelease(
-				model.NewRepository(1, model.Github, repositoryName).AddVersion(model.DefaultPattern, "1.1.0"),
+				model.NewRepository(1, model.Github, repositoryName).AddVersion(model.DefaultPattern, repositoryVersion),
 				model.DefaultPattern,
 				safeParse("1.1.0"),
 			)},
@@ -174,7 +159,7 @@ func TestGetNewReleases(t *testing.T) {
 	}
 }
 
-func TestCheckRepositoryVersion(t *testing.T) {
+func TestGetNewRepositoryReleases(t *testing.T) {
 	type args struct {
 		repo model.Repository
 	}
@@ -249,8 +234,8 @@ func TestCheckRepositoryVersion(t *testing.T) {
 
 	for _, tc := range cases {
 		t.Run(tc.intention, func(t *testing.T) {
-			if got := tc.instance.checkRepositoryVersion(tc.args.repo); !reflect.DeepEqual(got, tc.want) {
-				t.Errorf("checkRepositoryVersion() = %+v, want %+v", got, tc.want)
+			if got := tc.instance.getNewRepositoryReleases(tc.args.repo); !reflect.DeepEqual(got, tc.want) {
+				t.Errorf("getNewRepositoryReleases() = %+v, want %+v", got, tc.want)
 			}
 		})
 	}