From c281e808d6ba31d30c972c0f7d9bb11d1f5beedb Mon Sep 17 00:00:00 2001 From: Jim Date: Fri, 2 Apr 2021 19:13:48 -0400 Subject: [PATCH] refactor to use WithOrderByCreateTime(...) (#1058) --- internal/session/options.go | 24 ++++++++++--------- internal/session/options_test.go | 7 +++--- internal/session/repository.go | 7 ++++-- .../session/repository_connection_test.go | 2 +- internal/session/repository_session.go | 10 +++++--- internal/session/repository_session_test.go | 4 ++-- 6 files changed, 32 insertions(+), 22 deletions(-) diff --git a/internal/session/options.go b/internal/session/options.go index d5869f05f..c80b7dd8c 100644 --- a/internal/session/options.go +++ b/internal/session/options.go @@ -1,6 +1,7 @@ package session import ( + "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/boundary/internal/db/timestamp" ) @@ -18,14 +19,14 @@ type Option func(*options) // options = how options are represented type options struct { - withLimit int - withOrder string - withScopeIds []string - withUserId string - withExpirationTime *timestamp.Timestamp - withTestTofu []byte - withListingConvert bool - withSessionIds []string + withLimit int + withOrderByCreateTime db.OrderBy + withScopeIds []string + withUserId string + withExpirationTime *timestamp.Timestamp + withTestTofu []byte + withListingConvert bool + withSessionIds []string } func getDefaultOptions() options { @@ -41,10 +42,11 @@ func WithLimit(limit int) Option { } } -// WithOrder allows specifying an order for returned values -func WithOrder(order string) Option { +// WithOrderByCreateTime provides an option to specify ordering by the +// CreateTime field. +func WithOrderByCreateTime(orderBy db.OrderBy) Option { return func(o *options) { - o.withOrder = order + o.withOrderByCreateTime = orderBy } } diff --git a/internal/session/options_test.go b/internal/session/options_test.go index 009e4bd00..3b890d52d 100644 --- a/internal/session/options_test.go +++ b/internal/session/options_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/golang/protobuf/ptypes" + "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/boundary/internal/db/timestamp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -38,11 +39,11 @@ func Test_GetOpts(t *testing.T) { testOpts.withScopeIds = []string{"o_1234"} assert.Equal(opts, testOpts) }) - t.Run("WithOrder", func(t *testing.T) { + t.Run("WithOrderByCreateTime", func(t *testing.T) { assert := assert.New(t) - opts := getOpts(WithOrder("create_time asc")) + opts := getOpts(WithOrderByCreateTime(db.AscendingOrderBy)) testOpts := getDefaultOptions() - testOpts.withOrder = "create_time asc" + testOpts.withOrderByCreateTime = db.AscendingOrderBy assert.Equal(opts, testOpts) }) t.Run("WithUserId", func(t *testing.T) { diff --git a/internal/session/repository.go b/internal/session/repository.go index 169c69718..9c0dce910 100644 --- a/internal/session/repository.go +++ b/internal/session/repository.go @@ -61,8 +61,11 @@ func (r *Repository) list(ctx context.Context, resources interface{}, where stri limit = opts.withLimit } dbOpts = append(dbOpts, db.WithLimit(limit)) - if opts.withOrder != "" { - dbOpts = append(dbOpts, db.WithOrder(opts.withOrder)) + switch opts.withOrderByCreateTime { + case db.AscendingOrderBy: + dbOpts = append(dbOpts, db.WithOrder("create_time asc")) + case db.DescendingOrderBy: + dbOpts = append(dbOpts, db.WithOrder("create_time")) } if err := r.reader.SearchWhere(ctx, resources, where, args, dbOpts...); err != nil { return errors.Wrap(err, op) diff --git a/internal/session/repository_connection_test.go b/internal/session/repository_connection_test.go index dcd9add64..b5a35a5c8 100644 --- a/internal/session/repository_connection_test.go +++ b/internal/session/repository_connection_test.go @@ -115,7 +115,7 @@ func TestRepository_ListConnection(t *testing.T) { 2222, ) } - got, err := repo.ListConnections(context.Background(), session.PublicId, WithOrder("create_time asc")) + got, err := repo.ListConnections(context.Background(), session.PublicId, WithOrderByCreateTime(db.AscendingOrderBy)) require.NoError(err) assert.Equal(wantCnt, len(got)) diff --git a/internal/session/repository_session.go b/internal/session/repository_session.go index 531aa339c..af0b15ee5 100644 --- a/internal/session/repository_session.go +++ b/internal/session/repository_session.go @@ -210,8 +210,12 @@ func (r *Repository) ListSessions(ctx context.Context, opt ...Option) ([]*Sessio limit = fmt.Sprintf("limit %d", opts.withLimit) } - if opts.withOrder != "" { - opts.withOrder = fmt.Sprintf("order by %s", opts.withOrder) + var withOrder string + switch opts.withOrderByCreateTime { + case db.AscendingOrderBy: + withOrder = "order by create_time asc" + case db.DescendingOrderBy: + withOrder = "order by create_time" } var whereClause string @@ -219,7 +223,7 @@ func (r *Repository) ListSessions(ctx context.Context, opt ...Option) ([]*Sessio whereClause = " and " + strings.Join(where, " and ") } q := sessionList - query := fmt.Sprintf(q, limit, whereClause, opts.withOrder) + query := fmt.Sprintf(q, limit, whereClause, withOrder) rows, err := r.reader.Query(ctx, query, args) if err != nil { diff --git a/internal/session/repository_session_test.go b/internal/session/repository_session_test.go index bcfc37cfb..b54876e06 100644 --- a/internal/session/repository_session_test.go +++ b/internal/session/repository_session_test.go @@ -124,7 +124,7 @@ func TestRepository_ListSession(t *testing.T) { for i := 0; i < wantCnt; i++ { _ = TestSession(t, conn, wrapper, composedOf) } - got, err := repo.ListSessions(context.Background(), WithOrder("create_time asc")) + got, err := repo.ListSessions(context.Background(), WithOrderByCreateTime(db.AscendingOrderBy)) require.NoError(err) assert.Equal(wantCnt, len(got)) @@ -183,7 +183,7 @@ func TestRepository_ListSession(t *testing.T) { } assert.Equal(10, len(testSessions)) withIds := []string{testSessions[0].PublicId, testSessions[1].PublicId} - got, err := repo.ListSessions(context.Background(), WithSessionIds(withIds...), WithOrder("create_time asc")) + got, err := repo.ListSessions(context.Background(), WithSessionIds(withIds...), WithOrderByCreateTime(db.AscendingOrderBy)) require.NoError(err) assert.Equal(2, len(got)) assert.Equal(StatusActive, got[0].States[0].Status)