Skip to content

Commit

Permalink
feat(ketchup): Adding support for multiple pattern for same repository
Browse files Browse the repository at this point in the history 
Closes None

Signed-off-by: Vincent Boutour <[email protected]>
  • Loading branch information
@ViBiOh
ViBiOh committed May 27, 2021
1 parent d490ff8 commit d672509
Show file tree
Hide file tree
Showing 9 changed files with 80 additions and 56 deletions.
3 changes: 2 additions & 1 deletion cmd/ketchup/templates/ketchup.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,10 @@ <h2 class="header">Confirmation</h2>

<form method="POST" action="/app/ketchups/{{ .Repository.ID }}">
<input type="hidden" name="method" value="DELETE">
<input type="hidden" name="pattern" value="{{ .Pattern }}">

<p class="padding no-margin center">
Are you sure you want to delete <strong>{{ .Repository.Name }}{{ if eq .Repository.Kind.String "helm" }} | {{ .Repository.Part }}{{ end }}</strong>?
Are you sure you want to delete <strong>{{ .Repository.Name }}{{ if eq .Repository.Kind.String "helm" }} | {{ .Repository.Part }}{{ end }}</strong> for <strong>{{ .Pattern }}</strong>?
</p>

{{ template "form_buttons" "Delete" }}
Expand Down
2 changes: 1 addition & 1 deletion pkg/ketchup/ketchups.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (a app) handleDelete(w http.ResponseWriter, r *http.Request) {
return
}

item := model.NewKetchup("", "", model.Daily, model.NewGithubRepository(id, ""))
item := model.NewKetchup(r.FormValue("pattern"), "", model.Daily, model.NewGithubRepository(id, ""))

if err := a.ketchupService.Delete(r.Context(), item); err != nil {
a.rendererApp.Error(w, err)
Expand Down
20 changes: 14 additions & 6 deletions pkg/service/ketchup/ketchup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,15 @@ func (a app) Update(ctx context.Context, item model.Ketchup) (model.Ketchup, err
var output model.Ketchup

err := a.ketchupStore.DoAtomic(ctx, func(ctx context.Context) error {
old, err := a.ketchupStore.GetByRepositoryID(ctx, item.Repository.ID, true)
old, err := a.ketchupStore.GetByRepository(ctx, item.Repository.ID, item.Pattern, true)
if err != nil {
return httpModel.WrapInternal(fmt.Errorf("unable to fetch: %w", err))
}

if old.Repository.ID == 0 {
return httpModel.WrapNotFound(errors.New("unable to found repository"))
}

current := model.Ketchup{
Pattern: item.Pattern,
Version: item.Version,
Expand All @@ -128,7 +132,7 @@ func (a app) Update(ctx context.Context, item model.Ketchup) (model.Ketchup, err
current.Repository = repo
}

if err := a.ketchupStore.Update(ctx, current); err != nil {
if err := a.ketchupStore.Update(ctx, current, old.Pattern); err != nil {
return httpModel.WrapInternal(fmt.Errorf("unable to update: %w", err))
}

Expand All @@ -141,11 +145,15 @@ func (a app) Update(ctx context.Context, item model.Ketchup) (model.Ketchup, err

func (a app) Delete(ctx context.Context, item model.Ketchup) (err error) {
return a.ketchupStore.DoAtomic(ctx, func(ctx context.Context) error {
old, err := a.ketchupStore.GetByRepositoryID(ctx, item.Repository.ID, true)
old, err := a.ketchupStore.GetByRepository(ctx, item.Repository.ID, item.Pattern, true)
if err != nil {
return httpModel.WrapInternal(fmt.Errorf("unable to fetch current: %w", err))
}

if old.Repository.ID == 0 {
return httpModel.WrapNotFound(errors.New("unable to found repository"))
}

if err = a.check(ctx, old, model.NoneKetchup); err != nil {
return httpModel.WrapInvalid(err)
}
Expand Down Expand Up @@ -179,12 +187,12 @@ func (a app) check(ctx context.Context, old, new model.Ketchup) error {
output = append(output, errors.New("version is required"))
}

if old.Repository.ID == 0 && old.User.ID == 0 {
o, err := a.ketchupStore.GetByRepositoryID(ctx, new.Repository.ID, false)
if old.Repository.ID == 0 && new.Repository.ID != 0 {
o, err := a.ketchupStore.GetByRepository(ctx, new.Repository.ID, new.Pattern, false)
if err != nil {
output = append(output, errors.New("unable to check if ketchup already exists"))
} else if o.Repository.ID != 0 {
output = append(output, fmt.Errorf("ketchup for %s already exists", new.Repository.Name))
output = append(output, fmt.Errorf("ketchup for `%s` with pattern `%s` already exists", new.Repository.Name, new.Pattern))
}
}

Expand Down
28 changes: 14 additions & 14 deletions pkg/service/ketchup/ketchup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ func TestUpdate(t *testing.T) {
},
{
"fetch error",
New(ketchuptest.New().SetGetByRepositoryID(model.NoneKetchup, errors.New("failed")), repositorytest.New()),
New(ketchuptest.New().SetGetByRepository(model.NoneKetchup, errors.New("failed")), repositorytest.New()),
args{
ctx: context.Background(),
item: model.NoneKetchup,
Expand All @@ -257,7 +257,7 @@ func TestUpdate(t *testing.T) {
},
{
"check error",
New(ketchuptest.New().SetGetByRepositoryID(model.NewKetchup(model.DefaultPattern, "0.9.0", model.Daily, model.NewGithubRepository(1, ketchupRepository)), nil), repositorytest.New()),
New(ketchuptest.New().SetGetByRepository(model.NewKetchup(model.DefaultPattern, "0.9.0", model.Daily, model.NewGithubRepository(1, ketchupRepository)), nil), repositorytest.New()),
args{
ctx: context.Background(),
item: model.NewKetchup(model.DefaultPattern, "", model.Daily, model.NewGithubRepository(1, ketchupRepository)),
Expand All @@ -267,7 +267,7 @@ func TestUpdate(t *testing.T) {
},
{
"pattern change error",
New(ketchuptest.New().SetGetByRepositoryID(model.NewKetchup(model.DefaultPattern, "0.9.0", model.Daily, model.NewGithubRepository(1, ketchupRepository)), nil), repositorytest.New().SetGetOrCreate(model.NoneRepository, errors.New("failed"))),
New(ketchuptest.New().SetGetByRepository(model.NewKetchup(model.DefaultPattern, "0.9.0", model.Daily, model.NewGithubRepository(1, ketchupRepository)), nil), repositorytest.New().SetGetOrCreate(model.NoneRepository, errors.New("failed"))),
args{
ctx: model.StoreUser(context.Background(), model.NewUser(1, "", authModel.NewUser(0, ""))),
item: model.NewKetchup("latest", "1.0.0", model.Daily, model.NewGithubRepository(1, ketchupRepository)),
Expand All @@ -277,7 +277,7 @@ func TestUpdate(t *testing.T) {
},
{
"pattern change success",
New(ketchuptest.New().SetGetByRepositoryID(model.Ketchup{
New(ketchuptest.New().SetGetByRepository(model.Ketchup{
Pattern: model.DefaultPattern,
Version: "0.9.0",
Frequency: model.Daily,
Expand All @@ -299,7 +299,7 @@ func TestUpdate(t *testing.T) {
},
{
"update error",
New(ketchuptest.New().SetGetByRepositoryID(model.NewKetchup(model.DefaultPattern, "0.9.0", model.Daily, model.NewGithubRepository(1, ketchupRepository)), nil).SetUpdate(errors.New("failed")), repositorytest.New()),
New(ketchuptest.New().SetGetByRepository(model.NewKetchup(model.DefaultPattern, "0.9.0", model.Daily, model.NewGithubRepository(1, ketchupRepository)), nil).SetUpdate(errors.New("failed")), repositorytest.New()),
args{
ctx: model.StoreUser(context.Background(), model.NewUser(1, "", authModel.NewUser(0, ""))),
item: model.NewKetchup(model.DefaultPattern, "0.0.0", model.Daily, model.NewGithubRepository(2, "")),
Expand All @@ -309,7 +309,7 @@ func TestUpdate(t *testing.T) {
},
{
"success",
New(ketchuptest.New().SetGetByRepositoryID(model.Ketchup{
New(ketchuptest.New().SetGetByRepository(model.Ketchup{
Pattern: model.DefaultPattern,
Version: "0.9.0",
Frequency: model.Daily,
Expand Down Expand Up @@ -373,7 +373,7 @@ func TestDelete(t *testing.T) {
},
{
"fetch error",
New(ketchuptest.New().SetGetByRepositoryID(model.NoneKetchup, errors.New("failed")), repositorytest.New()),
New(ketchuptest.New().SetGetByRepository(model.NoneKetchup, errors.New("failed")), repositorytest.New()),
args{
ctx: context.Background(),
item: model.NewKetchup(model.DefaultPattern, "", model.Daily, model.NewGithubRepository(0, "")),
Expand All @@ -382,7 +382,7 @@ func TestDelete(t *testing.T) {
},
{
"check error",
New(ketchuptest.New(), repositorytest.New()),
New(ketchuptest.New().SetGetByRepository(model.NewKetchup(model.DefaultPattern, "", model.Daily, model.NewGithubRepository(1, "")), nil), repositorytest.New()),
args{
ctx: context.Background(),
item: model.NewKetchup(model.DefaultPattern, "", model.Daily, model.NewGithubRepository(1, "")),
Expand All @@ -391,7 +391,7 @@ func TestDelete(t *testing.T) {
},
{
"delete error",
New(ketchuptest.New().SetDelete(errors.New("failed")), repositorytest.New()),
New(ketchuptest.New().SetGetByRepository(model.NewKetchup(model.DefaultPattern, "", model.Daily, model.NewGithubRepository(1, "")), nil).SetDelete(errors.New("failed")), repositorytest.New()),
args{
ctx: model.StoreUser(context.Background(), model.NewUser(1, "", authModel.NewUser(0, ""))),
item: model.NewKetchup(model.DefaultPattern, "", model.Daily, model.NewGithubRepository(3, "")),
Expand All @@ -400,7 +400,7 @@ func TestDelete(t *testing.T) {
},
{
"success",
New(ketchuptest.New(), repositorytest.New()),
New(ketchuptest.New().SetGetByRepository(model.NewKetchup(model.DefaultPattern, "", model.Daily, model.NewGithubRepository(1, "")), nil), repositorytest.New()),
args{
ctx: model.StoreUser(context.Background(), model.NewUser(1, "", authModel.NewUser(0, ""))),
item: model.NewKetchup(model.DefaultPattern, "", model.Daily, model.NewGithubRepository(1, "")),
Expand Down Expand Up @@ -489,21 +489,21 @@ func TestCheck(t *testing.T) {
},
{
"create error",
app{ketchupStore: ketchuptest.New().SetGetByRepositoryID(model.NoneKetchup, errors.New("failed"))},
app{ketchupStore: ketchuptest.New().SetGetByRepository(model.NoneKetchup, errors.New("failed"))},
args{
ctx: model.StoreUser(context.Background(), model.NewUser(1, "", authModel.NewUser(0, ""))),
new: model.Ketchup{Version: "1.0.0", Repository: model.NewGithubRepository(0, ""), User: model.NewUser(1, "", authModel.NewUser(0, ""))},
new: model.Ketchup{Version: "1.0.0", Pattern: "stable", Repository: model.NewGithubRepository(1, ""), User: model.NewUser(1, "", authModel.NewUser(0, ""))},
},
errors.New("unable to check if ketchup already exists"),
},
{
"create already exists",
app{ketchupStore: ketchuptest.New().SetGetByRepositoryID(model.NewKetchup(model.DefaultPattern, "1.0.0", model.Daily, model.NewGithubRepository(1, ketchupRepository)), nil)},
app{ketchupStore: ketchuptest.New().SetGetByRepository(model.NewKetchup(model.DefaultPattern, "1.0.0", model.Daily, model.NewGithubRepository(1, ketchupRepository)), nil)},
args{
ctx: model.StoreUser(context.Background(), model.NewUser(1, "", authModel.NewUser(0, ""))),
new: model.Ketchup{Pattern: model.DefaultPattern, Version: "1.0.0", Repository: model.NewGithubRepository(2, ketchupRepository), User: model.NewUser(1, "", authModel.NewUser(0, ""))},
},
errors.New("ketchup for vibioh/ketchup already exists"),
errors.New("ketchup for `vibioh/ketchup` with pattern `stable` already exists"),
},
}

Expand Down
23 changes: 13 additions & 10 deletions pkg/store/ketchup/ketchup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ type App interface {
List(ctx context.Context, page uint, lastKey string) ([]model.Ketchup, uint64, error)
ListByRepositoriesID(ctx context.Context, ids []uint64, frequency model.KetchupFrequency) ([]model.Ketchup, error)
ListOutdatedByFrequency(ctx context.Context, frequency model.KetchupFrequency) ([]model.Ketchup, error)
GetByRepositoryID(ctx context.Context, id uint64, forUpdate bool) (model.Ketchup, error)
GetByRepository(ctx context.Context, id uint64, pattern string, forUpdate bool) (model.Ketchup, error)
Create(ctx context.Context, o model.Ketchup) (uint64, error)
Update(ctx context.Context, o model.Ketchup) error
Update(ctx context.Context, o model.Ketchup, oldPattern string) error
Delete(ctx context.Context, o model.Ketchup) error
}

Expand Down Expand Up @@ -242,10 +242,11 @@ FROM
WHERE
k.repository_id = $1
AND k.user_id = $2
AND k.pattern = $3
AND k.repository_id = r.id
`

func (a app) GetByRepositoryID(ctx context.Context, id uint64, forUpdate bool) (model.Ketchup, error) {
func (a app) GetByRepository(ctx context.Context, id uint64, pattern string, forUpdate bool) (model.Ketchup, error) {
query := getQuery
if forUpdate {
query += " FOR UPDATE"
Expand Down Expand Up @@ -282,7 +283,7 @@ func (a app) GetByRepositoryID(ctx context.Context, id uint64, forUpdate bool) (
return err
}

return item, db.Get(ctx, a.db, scanner, query, id, user.ID)
return item, db.Get(ctx, a.db, scanner, query, id, user.ID, pattern)
}

const insertQuery = `
Expand Down Expand Up @@ -311,16 +312,17 @@ const updateQuery = `
UPDATE
ketchup.ketchup
SET
pattern = $3,
version = $4,
frequency = $5
pattern = $4,
version = $5,
frequency = $6
WHERE
repository_id = $1
AND user_id = $2
AND pattern = $3
`

func (a app) Update(ctx context.Context, o model.Ketchup) error {
return db.Exec(ctx, updateQuery, o.Repository.ID, model.ReadUser(ctx).ID, o.Pattern, o.Version, strings.ToLower(o.Frequency.String()))
func (a app) Update(ctx context.Context, o model.Ketchup, oldPattern string) error {
return db.Exec(ctx, updateQuery, o.Repository.ID, model.ReadUser(ctx).ID, oldPattern, o.Pattern, o.Version, strings.ToLower(o.Frequency.String()))
}

const deleteQuery = `
Expand All @@ -329,8 +331,9 @@ DELETE FROM
WHERE
repository_id = $1
AND user_id = $2
AND pattern = $3
`

func (a app) Delete(ctx context.Context, o model.Ketchup) error {
return db.Exec(ctx, deleteQuery, o.Repository.ID, model.ReadUser(ctx).ID)
return db.Exec(ctx, deleteQuery, o.Repository.ID, model.ReadUser(ctx).ID, o.Pattern)
}
35 changes: 21 additions & 14 deletions pkg/store/ketchup/ketchup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,9 +261,10 @@ func TestListByRepositoriesID(t *testing.T) {
}
}

func TestGetByRepositoryID(t *testing.T) {
func TestGetByRepository(t *testing.T) {
type args struct {
id uint64
pattern string
forUpdate bool
}

Expand All @@ -277,9 +278,10 @@ func TestGetByRepositoryID(t *testing.T) {
{
"simple",
args{
id: 1,
id: 1,
pattern: "stable",
},
"SELECT k.pattern, k.version, k.frequency, k.repository_id, k.user_id, r.name, r.part, r.kind FROM ketchup.ketchup k, ketchup.repository r WHERE k.repository_id = .+ AND k.user_id = .+ AND k.repository_id = r.id",
"SELECT k.pattern, k.version, k.frequency, k.repository_id, k.user_id, r.name, r.part, r.kind FROM ketchup.ketchup k, ketchup.repository r WHERE k.repository_id = .+ AND k.user_id = .+ AND k.pattern = .+ AND k.repository_id = r.id",
model.Ketchup{
Pattern: model.DefaultPattern,
Version: "0.9.0",
Expand All @@ -292,19 +294,21 @@ func TestGetByRepositoryID(t *testing.T) {
{
"no rows",
args{
id: 1,
id: 1,
pattern: "stable",
},
"SELECT k.pattern, k.version, k.frequency, k.repository_id, k.user_id, r.name, r.part, r.kind FROM ketchup.ketchup k, ketchup.repository r WHERE k.repository_id = .+ AND k.user_id = .+ AND k.repository_id = r.id",
"SELECT k.pattern, k.version, k.frequency, k.repository_id, k.user_id, r.name, r.part, r.kind FROM ketchup.ketchup k, ketchup.repository r WHERE k.repository_id = .+ AND k.user_id = .+ AND k.pattern = .+ AND k.repository_id = r.id",
model.NoneKetchup,
nil,
},
{
"for update",
args{
id: 1,
pattern: "stable",
forUpdate: true,
},
"SELECT k.pattern, k.version, k.frequency, k.repository_id, k.user_id, r.name, r.part, r.kind FROM ketchup.ketchup k, ketchup.repository r WHERE k.repository_id = .+ AND k.user_id = .+ AND k.repository_id = r.id FOR UPDATE",
"SELECT k.pattern, k.version, k.frequency, k.repository_id, k.user_id, r.name, r.part, r.kind FROM ketchup.ketchup k, ketchup.repository r WHERE k.repository_id = .+ AND k.user_id = .+ AND k.pattern = .+ AND k.repository_id = r.id FOR UPDATE",
model.Ketchup{
Pattern: model.DefaultPattern,
Version: "0.9.0",
Expand All @@ -319,8 +323,8 @@ func TestGetByRepositoryID(t *testing.T) {
for _, tc := range cases {
t.Run(tc.intention, func(t *testing.T) {
testWithMock(t, func(mockDb *sql.DB, mock sqlmock.Sqlmock) {
rows := sqlmock.NewRows([]string{"patter", "version", "frequency", "repository_id", "user_id", "name", "part", "kind"})
mock.ExpectQuery(tc.expectSQL).WithArgs(1, 3).WillReturnRows(rows)
rows := sqlmock.NewRows([]string{"pattern", "version", "frequency", "repository_id", "user_id", "name", "part", "kind"})
mock.ExpectQuery(tc.expectSQL).WithArgs(1, 3, tc.args.pattern).WillReturnRows(rows)

switch tc.intention {
case "for update":
Expand All @@ -330,7 +334,7 @@ func TestGetByRepositoryID(t *testing.T) {
rows.AddRow(model.DefaultPattern, "0.9.0", "Daily", 1, 3, repositoryName, "", "github")
}

got, gotErr := New(mockDb).GetByRepositoryID(testCtx, tc.args.id, tc.args.forUpdate)
got, gotErr := New(mockDb).GetByRepository(testCtx, tc.args.id, tc.args.pattern, tc.args.forUpdate)

failed := false

Expand All @@ -341,7 +345,7 @@ func TestGetByRepositoryID(t *testing.T) {
}

if failed {
t.Errorf("GetByRepositoryID() = (%+v, `%s`), want (%+v, `%s`)", got, gotErr, tc.want, tc.wantErr)
t.Errorf("GetByRepository() = (%+v, `%s`), want (%+v, `%s`)", got, gotErr, tc.want, tc.wantErr)
}
})
})
Expand Down Expand Up @@ -401,7 +405,8 @@ func TestCreate(t *testing.T) {

func TestUpdate(t *testing.T) {
type args struct {
o model.Ketchup
o model.Ketchup
oldPattern string
}

var cases = []struct {
Expand All @@ -418,6 +423,7 @@ func TestUpdate(t *testing.T) {
Frequency: model.Daily,
Repository: model.NewGithubRepository(1, ""),
},
oldPattern: "stable",
},
nil,
},
Expand All @@ -428,9 +434,9 @@ func TestUpdate(t *testing.T) {
testWithMock(t, func(mockDb *sql.DB, mock sqlmock.Sqlmock) {
ctx := testWithTransaction(t, mockDb, mock)

mock.ExpectExec("UPDATE ketchup.ketchup SET pattern = .+, version = .+").WithArgs(1, 3, model.DefaultPattern, "0.9.0", "daily").WillReturnResult(sqlmock.NewResult(0, 1))
mock.ExpectExec("UPDATE ketchup.ketchup SET pattern = .+, version = .+").WithArgs(1, 3, tc.args.oldPattern, model.DefaultPattern, "0.9.0", "daily").WillReturnResult(sqlmock.NewResult(0, 1))

gotErr := New(mockDb).Update(ctx, tc.args.o)
gotErr := New(mockDb).Update(ctx, tc.args.o, tc.args.oldPattern)

failed := false

Expand Down Expand Up @@ -460,6 +466,7 @@ func TestDelete(t *testing.T) {
"simple",
args{
o: model.Ketchup{
Pattern: "stable",
Repository: model.NewGithubRepository(1, ""),
},
},
Expand All @@ -472,7 +479,7 @@ func TestDelete(t *testing.T) {
testWithMock(t, func(mockDb *sql.DB, mock sqlmock.Sqlmock) {
ctx := testWithTransaction(t, mockDb, mock)

mock.ExpectExec("DELETE FROM ketchup.ketchup").WithArgs(1, 3).WillReturnResult(sqlmock.NewResult(0, 1))
mock.ExpectExec("DELETE FROM ketchup.ketchup").WithArgs(1, 3, "stable").WillReturnResult(sqlmock.NewResult(0, 1))

gotErr := New(mockDb).Delete(ctx, tc.args.o)

Expand Down
Loading

0 comments on commit d672509

Please sign in to comment.