refactor(repository): Reducing cyclomatic complexity and extra sort

Signed-off-by: Vincent Boutour <[email protected]>
ViBiOh · Jun 14, 2021 · 6ff4c6a · 6ff4c6a
1 parent 8be7da9
commit 6ff4c6a
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 113 deletions.
diff --git a/pkg/model/repository.go b/pkg/model/repository.go
@@ -151,22 +151,3 @@ func (r Repository) CompareURL(version string, pattern string) string {
 		return r.URL(pattern)
 	}
 }
-
-// RepositoryByID sort repository by ID
-type RepositoryByID []Repository
-
-func (a RepositoryByID) Len() int           { return len(a) }
-func (a RepositoryByID) Less(i, j int) bool { return a[i].ID < a[j].ID }
-func (a RepositoryByID) Swap(i, j int)      { a[i], a[j] = a[j], a[i] }
-
-// RepositoryByName sort repository by Name
-type RepositoryByName []Repository
-
-func (a RepositoryByName) Len() int { return len(a) }
-func (a RepositoryByName) Less(i, j int) bool {
-	if a[i].Name == a[j].Name {
-		return a[i].Part < a[j].Part
-	}
-	return a[i].Name < a[j].Name
-}
-func (a RepositoryByName) Swap(i, j int) { a[i], a[j] = a[j], a[i] }
diff --git a/pkg/model/repository_test.go b/pkg/model/repository_test.go
@@ -2,8 +2,6 @@ package model
 
 import (
 	"errors"
-	"reflect"
-	"sort"
 	"strings"
 	"testing"
 )
@@ -170,75 +168,3 @@ func TestParseRepositoryKind(t *testing.T) {
 		})
 	}
 }
-
-func TestRepositoryByID(t *testing.T) {
-	type args struct {
-		array []Repository
-	}
-
-	var cases = []struct {
-		intention string
-		args      args
-		want      []Repository
-	}{
-		{
-			"simple",
-			args{
-				array: []Repository{
-					NewGithubRepository(10, ""),
-					NewGithubRepository(1, ""),
-				},
-			},
-			[]Repository{
-				NewGithubRepository(1, ""),
-				NewGithubRepository(10, ""),
-			},
-		},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.intention, func(t *testing.T) {
-			sort.Sort(RepositoryByID(tc.args.array))
-			if got := tc.args.array; !reflect.DeepEqual(got, tc.want) {
-				t.Errorf("KetchupByRepositoryID() = %+v, want %+v", got, tc.want)
-			}
-		})
-	}
-}
-
-func TestRepositoryByName(t *testing.T) {
-	type args struct {
-		array []Repository
-	}
-
-	var cases = []struct {
-		intention string
-		args      args
-		want      []Repository
-	}{
-		{
-			"simple",
-			args{
-				array: []Repository{
-					NewHelmRepository(3, "def", "abc"),
-					NewHelmRepository(2, "abc", "def"),
-					NewHelmRepository(1, "abc", "abc"),
-				},
-			},
-			[]Repository{
-				NewHelmRepository(1, "abc", "abc"),
-				NewHelmRepository(2, "abc", "def"),
-				NewHelmRepository(3, "def", "abc"),
-			},
-		},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.intention, func(t *testing.T) {
-			sort.Sort(RepositoryByName(tc.args.array))
-			if got := tc.args.array; !reflect.DeepEqual(got, tc.want) {
-				t.Errorf("RepositoryByName() = %+v, want %+v", got, tc.want)
-			}
-		})
-	}
-}
diff --git a/pkg/store/repository/repository.go b/pkg/store/repository/repository.go
@@ -5,7 +5,6 @@ import (
 	"database/sql"
 	"errors"
 	"fmt"
-	"sort"
 	"strconv"
 	"strings"
 
@@ -196,7 +195,6 @@ func (a app) ListByKinds(ctx context.Context, pageSize uint, last string, kinds
 	query.WriteString(fmt.Sprintf(" LIMIT $%d", len(queryArgs)))
 
 	list, count, err := a.list(ctx, query.String(), queryArgs...)
-	sort.Sort(model.RepositoryByName(list))
 
 	return list, count, err
 }

diff --git a/pkg/store/repository/repository_version.go b/pkg/store/repository/repository_version.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"database/sql"
 	"fmt"
-	"sort"
 
 	"github.com/ViBiOh/ketchup/pkg/model"
 	"github.com/lib/pq"
@@ -34,9 +33,8 @@ func (a app) enrichRepositoriesVersions(ctx context.Context, repositories []mode
 		ids[index] = repository.ID
 	}
 
-	sort.Sort(model.RepositoryByID(repositories))
+	var repository model.Repository
 
-	index := 0
 	scanner := func(rows *sql.Rows) error {
 		var repositoryID uint64
 		var pattern, version string
@@ -45,17 +43,28 @@ func (a app) enrichRepositoriesVersions(ctx context.Context, repositories []mode
 			return err
 		}
 
-		for ; repositoryID != repositories[index].ID; index++ {
+		if repository.ID != repositoryID {
+			repository = findRepository(repositories, repositoryID)
 		}
 
-		repositories[index].Versions[pattern] = version
+		repository.Versions[pattern] = version
 
 		return nil
 	}
 
 	return a.db.List(ctx, scanner, listRepositoryVersionsForIDsQuery, pq.Array(ids))
 }
 
+func findRepository(repositories []model.Repository, id uint64) model.Repository {
+	for _, repo := range repositories {
+		if repo.ID == id {
+			return repo
+		}
+	}
+
+	return model.NoneRepository
+}
+
 const listRepositoryVersionQuery = `
 SELECT
   pattern,
@@ -119,22 +128,30 @@ func (a app) UpdateVersions(ctx context.Context, o model.Repository) error {
 	}
 
 	for pattern, version := range patterns {
-		if repositoryVersion, ok := o.Versions[pattern]; ok {
-			if repositoryVersion != version {
-				if err := a.db.Exec(ctx, updateRepositoryVersionQuery, o.ID, pattern, repositoryVersion); err != nil {
-					return fmt.Errorf("unable to update repository version: %w", err)
-				}
+		repositoryVersion, ok := o.Versions[pattern]
+		if !ok {
+			if err := a.db.Exec(ctx, deleteRepositoryVersionQuery, o.ID, pattern); err != nil {
+				return fmt.Errorf("unable to delete repository version: %w", err)
 			}
-		} else if err := a.db.Exec(ctx, deleteRepositoryVersionQuery, o.ID, pattern); err != nil {
-			return fmt.Errorf("unable to delete repository version: %w", err)
+			continue
+		}
+
+		if repositoryVersion == version {
+			continue
+		}
+
+		if err := a.db.Exec(ctx, updateRepositoryVersionQuery, o.ID, pattern, repositoryVersion); err != nil {
+			return fmt.Errorf("unable to update repository version: %w", err)
 		}
 	}
 
 	for pattern, version := range o.Versions {
-		if _, ok := patterns[pattern]; !ok {
-			if err := a.db.Exec(ctx, createRepositoryVersionQuery, o.ID, pattern, version); err != nil {
-				return fmt.Errorf("unable to create repository version: %w", err)
-			}
+		if _, ok := patterns[pattern]; ok {
+			continue
+		}
+
+		if err := a.db.Exec(ctx, createRepositoryVersionQuery, o.ID, pattern, version); err != nil {
+			return fmt.Errorf("unable to create repository version: %w", err)
 		}
 	}
 

diff --git a/pkg/store/repository/repository_version_test.go b/pkg/store/repository/repository_version_test.go
@@ -68,8 +68,8 @@ func TestEnrichRepositoriesVersions(t *testing.T) {
 				},
 			},
 			[]model.Repository{
-				model.NewGithubRepository(1, "").AddVersion(model.DefaultPattern, "1.0.0"),
 				model.NewGithubRepository(2, "").AddVersion(model.DefaultPattern, "1.1.0"),
+				model.NewGithubRepository(1, "").AddVersion(model.DefaultPattern, "1.0.0"),
 			},
 			nil,
 		},
@@ -92,9 +92,9 @@ func TestEnrichRepositoriesVersions(t *testing.T) {
 				},
 			},
 			[]model.Repository{
-				model.NewGithubRepository(1, "").AddVersion(model.DefaultPattern, "1.0.0").AddVersion("beta", "1.0.0-beta"),
 				model.NewGithubRepository(2, ""),
 				model.NewGithubRepository(3, "").AddVersion(model.DefaultPattern, "1.1.0").AddVersion("alpha", "2.0.0"),
+				model.NewGithubRepository(1, "").AddVersion(model.DefaultPattern, "1.0.0").AddVersion("beta", "1.0.0-beta"),
 			},
 			nil,
 		},