From 210f4d64eddb829499658218dbda3fa1ce20c652 Mon Sep 17 00:00:00 2001 From: noah Date: Sat, 9 Apr 2022 16:34:00 +0900 Subject: [PATCH 1/2] Reorder columns for (repo_id, number) index --- model/ent/migrate/schema.go | 4 ++-- model/ent/schema/deployment.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/model/ent/migrate/schema.go b/model/ent/migrate/schema.go index b67c7563..0eeea7f5 100644 --- a/model/ent/migrate/schema.go +++ b/model/ent/migrate/schema.go @@ -90,9 +90,9 @@ var ( Columns: []*schema.Column{DeploymentsColumns[16], DeploymentsColumns[12]}, }, { - Name: "deployment_number_repo_id", + Name: "deployment_repo_id_number", Unique: true, - Columns: []*schema.Column{DeploymentsColumns[1], DeploymentsColumns[16]}, + Columns: []*schema.Column{DeploymentsColumns[16], DeploymentsColumns[1]}, }, { Name: "deployment_uid", diff --git a/model/ent/schema/deployment.go b/model/ent/schema/deployment.go index e6a93b05..6f1a922e 100644 --- a/model/ent/schema/deployment.go +++ b/model/ent/schema/deployment.go @@ -107,7 +107,7 @@ func (Deployment) Indexes() []ent.Index { index.Fields("repo_id", "env", "created_at"), index.Fields("repo_id", "created_at"), // The deployment number is unique for the repo. - index.Fields("number", "repo_id"). + index.Fields("repo_id", "number"). Unique(), // Find by UID when the hook is coming. index.Fields("uid"), From c5a9167bf3cfe0db75d918b720b3e13a47267e96 Mon Sep 17 00:00:00 2001 From: noah Date: Sat, 9 Apr 2022 16:48:02 +0900 Subject: [PATCH 2/2] Fix the bug of returning the next number. --- internal/pkg/store/deployment.go | 27 ++++++++++++++++++++------- internal/pkg/store/deployment_test.go | 16 +++++----------- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/internal/pkg/store/deployment.go b/internal/pkg/store/deployment.go index 4c240378..da850eb7 100644 --- a/internal/pkg/store/deployment.go +++ b/internal/pkg/store/deployment.go @@ -196,16 +196,29 @@ func (s *Store) FindDeploymentByUID(ctx context.Context, uid int64) (*ent.Deploy } func (s *Store) GetNextDeploymentNumberOfRepo(ctx context.Context, r *ent.Repo) (int, error) { - cnt, err := s.c.Deployment.Query(). - Where( - deployment.RepoID(r.ID), - ). - Count(ctx) + var v []struct { + RepoID int64 `json:"repo_id"` + Max int `json:"max"` + } + err := s.c.Deployment.Query(). + Where(deployment.RepoID(r.ID)). + GroupBy(deployment.FieldRepoID). + Aggregate(ent.Max(deployment.FieldNumber)). + Scan(ctx, &v) if err != nil { - return 0, err + return 0, e.NewError(e.ErrorCodeInternalError, err) + } + + if len(v) >= 2 { + return 0, e.NewErrorWithMessage(e.ErrorCodeInternalError, "The result format of query is invalid.", err) + } + + // The return value must be one when there is no deployment record. + if len(v) == 0 { + return 1, nil } - return cnt + 1, nil + return v[0].Max + 1, nil } // FindPrevRunningDeployment find a deployment of which the status is created, queued, or running. diff --git a/internal/pkg/store/deployment_test.go b/internal/pkg/store/deployment_test.go index 21ec8772..988e208c 100644 --- a/internal/pkg/store/deployment_test.go +++ b/internal/pkg/store/deployment_test.go @@ -290,12 +290,6 @@ func TestStore_GetNextDeploymentNumberOfRepo(t *testing.T) { }) t.Run("Return two when there is a single deployment.", func(t *testing.T) { - const ( - u1 = 1 - r1 = 1 - r2 = 2 - ) - client := enttest.Open(t, "sqlite3", "file:ent?mode=memory&cache=shared&_fk=1", enttest.WithMigrateOptions(migrate.WithForeignKeys(false)), ) @@ -309,8 +303,8 @@ func TestStore_GetNextDeploymentNumberOfRepo(t *testing.T) { SetType("branch"). SetRef("main"). SetEnv("local"). - SetUserID(u1). - SetRepoID(r1). + SetUserID(1). + SetRepoID(1). SetStatus(deployment.StatusCreated). SaveX(ctx) @@ -320,14 +314,14 @@ func TestStore_GetNextDeploymentNumberOfRepo(t *testing.T) { SetType("branch"). SetRef("main"). SetEnv("prod"). - SetUserID(u1). - SetRepoID(r2). + SetUserID(1). + SetRepoID(2). SetStatus(deployment.StatusCreated). SaveX(ctx) s := NewStore(client) - number, err := s.GetNextDeploymentNumberOfRepo(ctx, &ent.Repo{ID: r1}) + number, err := s.GetNextDeploymentNumberOfRepo(ctx, &ent.Repo{ID: 1}) if err != nil { t.Fatalf("GetNextDeploymentNumberOfRepo returns an error: %s", err) t.FailNow()