From b8f6e6b39aee0dd8c10fe15b597a4009bee68e3a Mon Sep 17 00:00:00 2001 From: noah Date: Mon, 14 Mar 2022 22:25:22 +0900 Subject: [PATCH 1/5] Fix the bug of predicate --- internal/pkg/store/deployment.go | 31 ++++----- internal/pkg/store/deployment_test.go | 91 +++++++++++++-------------- 2 files changed, 56 insertions(+), 66 deletions(-) diff --git a/internal/pkg/store/deployment.go b/internal/pkg/store/deployment.go index da850eb7..aba34fb0 100644 --- a/internal/pkg/store/deployment.go +++ b/internal/pkg/store/deployment.go @@ -35,6 +35,7 @@ func (s *Store) SearchDeploymentsOfUser(ctx context.Context, u *ent.User, opt *i return deployment.StatusIn(ss...) } + // Search deployments that were triggered by the user. if opt.Owned { return s.c.Deployment. Query(). @@ -46,31 +47,25 @@ func (s *Store) SearchDeploymentsOfUser(ctx context.Context, u *ent.User, opt *i deployment.CreatedAtLT(opt.To), ), ). - WithRepo(). - WithUser(). Order(ent.Desc(deployment.FieldCreatedAt)). Offset(offset(opt.Page, opt.PerPage)). Limit(opt.PerPage). + WithRepo(). + WithUser(). All(ctx) } return s.c.Deployment. Query(). Where(func(s *sql.Selector) { - t := sql.Table(perm.Table) - - // Join with Perm for Repo.ID - s.Join(t). - On( - s.C(deployment.FieldRepoID), - s.C(perm.FieldRepoID), - ). - Where( - sql.EQ( - t.C(perm.FieldUserID), - u.ID, - ), - ) + p := sql.Table(perm.Table) + + s.Join(p).OnP( + sql.And( + sql.ColumnsEQ(p.C(perm.FieldRepoID), s.C(deployment.FieldRepoID)), + sql.EQ(p.C(perm.FieldUserID), u.ID), + ), + ) }). Where( deployment.And( @@ -79,11 +74,11 @@ func (s *Store) SearchDeploymentsOfUser(ctx context.Context, u *ent.User, opt *i deployment.CreatedAtLT(opt.To), ), ). - WithRepo(). - WithUser(). Order(ent.Desc(deployment.FieldCreatedAt)). Offset(offset(opt.Page, opt.PerPage)). Limit(opt.PerPage). + WithRepo(). + WithUser(). All(ctx) } diff --git a/internal/pkg/store/deployment_test.go b/internal/pkg/store/deployment_test.go index 988e208c..166172e4 100644 --- a/internal/pkg/store/deployment_test.go +++ b/internal/pkg/store/deployment_test.go @@ -23,26 +23,38 @@ func TestStore_SearchDeployments(t *testing.T) { ctx := context.Background() - const ( - u1 = 1 - u2 = 2 - r1 = 1 - r2 = 2 - ) + t.Log("User_1 has the permissions for repo_1 and repo_2.") + client.Perm. + Create(). + SetUserID(1). + SetRepoID(1). + SaveX(ctx) client.Perm. Create(). - SetUserID(u1). - SetRepoID(r1). + SetUserID(1). + SetRepoID(2). SaveX(ctx) + t.Log("Create the deployments under the repo_1.") client.Deployment.Create(). SetType(deployment.TypeBranch). SetNumber(1). SetRef("main"). SetEnv("local"). - SetUserID(u1). - SetRepoID(r1). + SetStatus(deployment.StatusCreated). + SetUserID(1). + SetRepoID(1). + SaveX(ctx) + + t.Log("Create the deployments under the repo_2.") + client.Deployment.Create(). + SetType(deployment.TypeBranch). + SetNumber(1). + SetRef("main"). + SetEnv("dev"). + SetUserID(1). + SetRepoID(2). SaveX(ctx) client.Deployment.Create(). @@ -50,35 +62,30 @@ func TestStore_SearchDeployments(t *testing.T) { SetNumber(2). SetRef("main"). SetEnv("dev"). - SetUserID(u2). - SetRepoID(r1). + SetUserID(2). + SetRepoID(2). SaveX(ctx) + t.Log("Create the deployments under the repo_3.") client.Deployment.Create(). SetType(deployment.TypeBranch). - SetNumber(3). + SetNumber(1). SetRef("main"). SetEnv("dev"). - SetUserID(u2). - SetRepoID(r1). + SetUserID(2). + SetRepoID(3). SetStatus(deployment.StatusCreated). SaveX(ctx) - t.Run("u1 searchs all deployments of r1.", func(t *testing.T) { - const ( - owned = false - page = 1 - perPage = 30 - ) - + t.Run("Returns all deployments under the accessible repositories.", func(t *testing.T) { store := NewStore(client) res, err := store.SearchDeploymentsOfUser(ctx, - &ent.User{ID: u1}, + &ent.User{ID: 1}, &i.SearchDeploymentsOfUserOptions{ - ListOptions: i.ListOptions{Page: page, PerPage: perPage}, + ListOptions: i.ListOptions{Page: 1, PerPage: 30}, Statuses: []deployment.Status{}, - Owned: owned, + Owned: false, From: time.Now().UTC().Add(-time.Minute), To: time.Now().UTC(), }) @@ -92,21 +99,15 @@ func TestStore_SearchDeployments(t *testing.T) { } }) - t.Run("u1 searchs waiting deployments of r1.", func(t *testing.T) { - const ( - owned = false - page = 1 - perPage = 30 - ) - + t.Run("Returns waiting deployments under the accessible repositories.", func(t *testing.T) { store := NewStore(client) res, err := store.SearchDeploymentsOfUser(ctx, - &ent.User{ID: u1}, + &ent.User{ID: 1}, &i.SearchDeploymentsOfUserOptions{ - ListOptions: i.ListOptions{Page: page, PerPage: perPage}, - Statuses: []deployment.Status{deployment.StatusWaiting}, - Owned: owned, + ListOptions: i.ListOptions{Page: 1, PerPage: 30}, + Statuses: []deployment.Status{deployment.StatusCreated}, + Owned: false, From: time.Now().UTC().Add(-time.Minute), To: time.Now().UTC(), }) @@ -114,27 +115,21 @@ func TestStore_SearchDeployments(t *testing.T) { t.Fatalf("SearchDeployments return an error: %s", err) } - expected := 2 + expected := 1 if len(res) != expected { t.Fatalf("SearchDeployments = %v, wanted %v", len(res), expected) } }) - t.Run("u1 searchs owned deployments of r1.", func(t *testing.T) { - const ( - owned = true - page = 1 - perPage = 30 - ) - + t.Run("Returns all deployment triggered by myself.", func(t *testing.T) { store := NewStore(client) res, err := store.SearchDeploymentsOfUser(ctx, - &ent.User{ID: u1}, + &ent.User{ID: 1}, &i.SearchDeploymentsOfUserOptions{ - ListOptions: i.ListOptions{Page: page, PerPage: perPage}, + ListOptions: i.ListOptions{Page: 1, PerPage: 30}, Statuses: []deployment.Status{}, - Owned: owned, + Owned: true, From: time.Now().UTC().Add(-time.Minute), To: time.Now().UTC(), }) @@ -142,7 +137,7 @@ func TestStore_SearchDeployments(t *testing.T) { t.Fatalf("SearchDeployments return an error: %s", err) } - expected := 1 + expected := 2 if len(res) != expected { t.Fatalf("SearchDeployments = %v, wanted %v", res, expected) } From b6a9a78851c1eafbce6caa5467ddb94e8c381020 Mon Sep 17 00:00:00 2001 From: noah Date: Mon, 14 Mar 2022 22:25:43 +0900 Subject: [PATCH 2/5] Fix the order of the index --- model/ent/migrate/schema.go | 2 +- model/ent/schema/deployment.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/model/ent/migrate/schema.go b/model/ent/migrate/schema.go index 0eeea7f5..5c25bc92 100644 --- a/model/ent/migrate/schema.go +++ b/model/ent/migrate/schema.go @@ -100,7 +100,7 @@ var ( Columns: []*schema.Column{DeploymentsColumns[7]}, }, { - Name: "deployment_status_created_at", + Name: "deployment_created_at_status", Unique: false, Columns: []*schema.Column{DeploymentsColumns[6], DeploymentsColumns[12]}, }, diff --git a/model/ent/schema/deployment.go b/model/ent/schema/deployment.go index 6f1a922e..d24dd310 100644 --- a/model/ent/schema/deployment.go +++ b/model/ent/schema/deployment.go @@ -112,6 +112,7 @@ func (Deployment) Indexes() []ent.Index { // Find by UID when the hook is coming. index.Fields("uid"), // List inactive deployments for 30 minutes. - index.Fields("status", "created_at"), + // Or search deployments that were created between the start time and the end time. + index.Fields("created_at", "status"), } } From b0c373665004b0ee1c201ac6f5cd174d4e3afebb Mon Sep 17 00:00:00 2001 From: noah Date: Mon, 14 Mar 2022 22:27:57 +0900 Subject: [PATCH 3/5] Set the format UTC at the store pkg --- internal/pkg/store/deployment.go | 8 ++++---- internal/server/api/v1/search/search.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/pkg/store/deployment.go b/internal/pkg/store/deployment.go index aba34fb0..e705315f 100644 --- a/internal/pkg/store/deployment.go +++ b/internal/pkg/store/deployment.go @@ -43,8 +43,8 @@ func (s *Store) SearchDeploymentsOfUser(ctx context.Context, u *ent.User, opt *i deployment.And( deployment.UserIDEQ(u.ID), statusIn(opt.Statuses), - deployment.CreatedAtGTE(opt.From), - deployment.CreatedAtLT(opt.To), + deployment.CreatedAtGTE(opt.From.UTC()), + deployment.CreatedAtLT(opt.To.UTC()), ), ). Order(ent.Desc(deployment.FieldCreatedAt)). @@ -70,8 +70,8 @@ func (s *Store) SearchDeploymentsOfUser(ctx context.Context, u *ent.User, opt *i Where( deployment.And( statusIn(opt.Statuses), - deployment.CreatedAtGTE(opt.From), - deployment.CreatedAtLT(opt.To), + deployment.CreatedAtGTE(opt.From.UTC()), + deployment.CreatedAtLT(opt.To.UTC()), ), ). Order(ent.Desc(deployment.FieldCreatedAt)). diff --git a/internal/server/api/v1/search/search.go b/internal/server/api/v1/search/search.go index 2f7d7261..b8bbd66b 100644 --- a/internal/server/api/v1/search/search.go +++ b/internal/server/api/v1/search/search.go @@ -115,8 +115,8 @@ func (s *Search) SearchDeployments(c *gin.Context) { ListOptions: i.ListOptions{Page: p, PerPage: pp}, Statuses: ss, Owned: o, - From: f.UTC(), - To: t.UTC(), + From: f, + To: t, }); err != nil { s.log.Check(gb.GetZapLogLevel(err), "Failed to search deployments.").Write(zap.Error(err)) gb.ResponseWithError(c, err) From 1e1891956e5ba92b0bee84be46a5c6afe253a156 Mon Sep 17 00:00:00 2001 From: noah Date: Mon, 14 Mar 2022 22:51:01 +0900 Subject: [PATCH 4/5] Add the `productionOnly` option --- internal/interactor/deployment.go | 9 ++-- internal/pkg/store/deployment.go | 38 ++++++-------- internal/pkg/store/deployment_test.go | 35 ++++++++++--- internal/server/api/v1/search/search.go | 69 ++++++++++--------------- 4 files changed, 77 insertions(+), 74 deletions(-) diff --git a/internal/interactor/deployment.go b/internal/interactor/deployment.go index c1ae9532..52ec514d 100644 --- a/internal/interactor/deployment.go +++ b/internal/interactor/deployment.go @@ -39,10 +39,11 @@ type ( SearchDeploymentsOfUserOptions struct { ListOptions - Statuses []deployment.Status - Owned bool - From time.Time - To time.Time + Statuses []deployment.Status + Owned bool + ProductionOnly bool + From time.Time + To time.Time } // ListInactiveDeploymentsLessThanTimeOptions specifies the optional parameters that diff --git a/internal/pkg/store/deployment.go b/internal/pkg/store/deployment.go index e705315f..901d3e69 100644 --- a/internal/pkg/store/deployment.go +++ b/internal/pkg/store/deployment.go @@ -35,27 +35,8 @@ func (s *Store) SearchDeploymentsOfUser(ctx context.Context, u *ent.User, opt *i return deployment.StatusIn(ss...) } - // Search deployments that were triggered by the user. - if opt.Owned { - return s.c.Deployment. - Query(). - Where( - deployment.And( - deployment.UserIDEQ(u.ID), - statusIn(opt.Statuses), - deployment.CreatedAtGTE(opt.From.UTC()), - deployment.CreatedAtLT(opt.To.UTC()), - ), - ). - Order(ent.Desc(deployment.FieldCreatedAt)). - Offset(offset(opt.Page, opt.PerPage)). - Limit(opt.PerPage). - WithRepo(). - WithUser(). - All(ctx) - } - - return s.c.Deployment. + // Build the query searching all deployments under the accessible repositories. + qry := s.c.Deployment. Query(). Where(func(s *sql.Selector) { p := sql.Table(perm.Table) @@ -78,8 +59,19 @@ func (s *Store) SearchDeploymentsOfUser(ctx context.Context, u *ent.User, opt *i Offset(offset(opt.Page, opt.PerPage)). Limit(opt.PerPage). WithRepo(). - WithUser(). - All(ctx) + WithUser() + + // Search only deployments that were triggered by the user. + if opt.Owned { + qry.Where(deployment.UserIDEQ(u.ID)) + } + + // Search only deployments for production environment. + if opt.ProductionOnly { + qry.Where(deployment.ProductionEnvironment(true)) + } + + return qry.All(ctx) } func (s *Store) ListInactiveDeploymentsLessThanTime(ctx context.Context, opt *i.ListInactiveDeploymentsLessThanTimeOptions) ([]*ent.Deployment, error) { diff --git a/internal/pkg/store/deployment_test.go b/internal/pkg/store/deployment_test.go index 166172e4..55b2e2fa 100644 --- a/internal/pkg/store/deployment_test.go +++ b/internal/pkg/store/deployment_test.go @@ -43,6 +43,7 @@ func TestStore_SearchDeployments(t *testing.T) { SetRef("main"). SetEnv("local"). SetStatus(deployment.StatusCreated). + SetProductionEnvironment(true). SetUserID(1). SetRepoID(1). SaveX(ctx) @@ -99,7 +100,29 @@ func TestStore_SearchDeployments(t *testing.T) { } }) - t.Run("Returns waiting deployments under the accessible repositories.", func(t *testing.T) { + t.Run("Returns all deployment triggered by myself.", func(t *testing.T) { + store := NewStore(client) + + res, err := store.SearchDeploymentsOfUser(ctx, + &ent.User{ID: 1}, + &i.SearchDeploymentsOfUserOptions{ + ListOptions: i.ListOptions{Page: 1, PerPage: 30}, + Statuses: []deployment.Status{}, + Owned: true, + From: time.Now().UTC().Add(-time.Minute), + To: time.Now().UTC(), + }) + if err != nil { + t.Fatalf("SearchDeployments return an error: %s", err) + } + + expected := 2 + if len(res) != expected { + t.Fatalf("SearchDeployments = %v, wanted %v", res, expected) + } + }) + + t.Run("Returns deployments which are 'created'.", func(t *testing.T) { store := NewStore(client) res, err := store.SearchDeploymentsOfUser(ctx, @@ -121,15 +144,15 @@ func TestStore_SearchDeployments(t *testing.T) { } }) - t.Run("Returns all deployment triggered by myself.", func(t *testing.T) { + t.Run("Returns deployments for production environment.", func(t *testing.T) { store := NewStore(client) res, err := store.SearchDeploymentsOfUser(ctx, &ent.User{ID: 1}, &i.SearchDeploymentsOfUserOptions{ ListOptions: i.ListOptions{Page: 1, PerPage: 30}, - Statuses: []deployment.Status{}, - Owned: true, + Statuses: []deployment.Status{deployment.StatusCreated}, + Owned: false, From: time.Now().UTC().Add(-time.Minute), To: time.Now().UTC(), }) @@ -137,9 +160,9 @@ func TestStore_SearchDeployments(t *testing.T) { t.Fatalf("SearchDeployments return an error: %s", err) } - expected := 2 + expected := 1 if len(res) != expected { - t.Fatalf("SearchDeployments = %v, wanted %v", res, expected) + t.Fatalf("SearchDeployments = %v, wanted %v", len(res), expected) } }) } diff --git a/internal/server/api/v1/search/search.go b/internal/server/api/v1/search/search.go index b8bbd66b..bf6cd14e 100644 --- a/internal/server/api/v1/search/search.go +++ b/internal/server/api/v1/search/search.go @@ -38,40 +38,32 @@ func (s *Search) SearchDeployments(c *gin.Context) { ctx := c.Request.Context() var ( - statuses = c.DefaultQuery("statuses", "") - owned = c.DefaultQuery("owned", "true") - from = c.DefaultQuery("from", time.Now().Add(-activeDuration).Format(time.RFC3339)) - to = c.DefaultQuery("to", time.Now().Format(time.RFC3339)) - page = c.DefaultQuery("page", "1") - perPage = c.DefaultQuery("per_page", "30") - ) - - var ( - ss = make([]deployment.Status, 0) - o bool - f time.Time - t time.Time - p int - pp int - err error + statuses = []deployment.Status{} + owned bool + productionOnly bool + from, to time.Time + page, perPage int + err error ) // Validate query parameters. - for _, st := range strings.Split(statuses, ",") { - if st != "" { - ss = append(ss, deployment.Status(st)) + for _, s := range strings.Split(c.DefaultQuery("statuses", ""), ",") { + if s != "" { + statuses = append(statuses, deployment.Status(s)) } } - if o, err = strconv.ParseBool(owned); err != nil { - gb.ResponseWithError( - c, - e.NewErrorWithMessage(e.ErrorCodeParameterInvalid, "The owned must be boolean.", err), - ) + if owned, err = strconv.ParseBool(c.DefaultQuery("owned", "true")); err != nil { + gb.ResponseWithError(c, e.NewErrorWithMessage(e.ErrorCodeParameterInvalid, "The owned must be boolean.", err)) + return + } + + if productionOnly, err = strconv.ParseBool(c.DefaultQuery("production_only", "false")); err != nil { + gb.ResponseWithError(c, e.NewErrorWithMessage(e.ErrorCodeParameterInvalid, "The production must be boolean.", err)) return } - if f, err = time.Parse(time.RFC3339, from); err != nil { + if from, err = time.Parse(time.RFC3339, c.DefaultQuery("from", time.Now().Add(-activeDuration).Format(time.RFC3339))); err != nil { gb.ResponseWithError( c, e.NewErrorWithMessage(e.ErrorCodeParameterInvalid, "Invalid format of \"from\" parameter, RFC3339 format only.", err), @@ -79,7 +71,7 @@ func (s *Search) SearchDeployments(c *gin.Context) { return } - if t, err = time.Parse(time.RFC3339, to); err != nil { + if to, err = time.Parse(time.RFC3339, c.DefaultQuery("to", time.Now().Format(time.RFC3339))); err != nil { gb.ResponseWithError( c, e.NewErrorWithMessage(e.ErrorCodeParameterInvalid, "Invalid format of \"to\" parameter, RFC3339 format only.", err), @@ -87,19 +79,13 @@ func (s *Search) SearchDeployments(c *gin.Context) { return } - if p, err = strconv.Atoi(page); err != nil { - gb.ResponseWithError( - c, - e.NewErrorWithMessage(e.ErrorCodeParameterInvalid, "Invalid format of \"page\" parameter.", err), - ) + if page, err = strconv.Atoi(c.DefaultQuery("page", "1")); err != nil { + gb.ResponseWithError(c, e.NewErrorWithMessage(e.ErrorCodeParameterInvalid, "The page must be number.", err)) return } - if pp, err = strconv.Atoi(perPage); err != nil { - gb.ResponseWithError( - c, - e.NewErrorWithMessage(e.ErrorCodeParameterInvalid, "Invalid format of \"per_page\" parameter.", err), - ) + if perPage, err = strconv.Atoi(c.DefaultQuery("per_page", "1")); err != nil { + gb.ResponseWithError(c, e.NewErrorWithMessage(e.ErrorCodeParameterInvalid, "The per_page must be number.", err)) return } @@ -112,11 +98,12 @@ func (s *Search) SearchDeployments(c *gin.Context) { u := v.(*ent.User) if ds, err = s.i.SearchDeploymentsOfUser(ctx, u, &i.SearchDeploymentsOfUserOptions{ - ListOptions: i.ListOptions{Page: p, PerPage: pp}, - Statuses: ss, - Owned: o, - From: f, - To: t, + ListOptions: i.ListOptions{Page: page, PerPage: perPage}, + Statuses: statuses, + Owned: owned, + ProductionOnly: productionOnly, + From: from, + To: to, }); err != nil { s.log.Check(gb.GetZapLogLevel(err), "Failed to search deployments.").Write(zap.Error(err)) gb.ResponseWithError(c, err) From 34dd81f2b949a0d1e793efdb12f91474e88c2d23 Mon Sep 17 00:00:00 2001 From: noah Date: Sun, 10 Apr 2022 14:50:42 +0900 Subject: [PATCH 5/5] Regenerate ent --- model/ent/migrate/schema.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/ent/migrate/schema.go b/model/ent/migrate/schema.go index 5c25bc92..587d1d18 100644 --- a/model/ent/migrate/schema.go +++ b/model/ent/migrate/schema.go @@ -102,7 +102,7 @@ var ( { Name: "deployment_created_at_status", Unique: false, - Columns: []*schema.Column{DeploymentsColumns[6], DeploymentsColumns[12]}, + Columns: []*schema.Column{DeploymentsColumns[12], DeploymentsColumns[6]}, }, }, }