From f21996d8adba4d6ba192855f08e2d0f72e8763a3 Mon Sep 17 00:00:00 2001 From: Jim Date: Fri, 2 Apr 2021 18:19:48 -0400 Subject: [PATCH] refactor to use WithOrderByVersion(...) (#1059) --- internal/db/read_writer.go | 15 +++++++++++++++ internal/kms/kms.go | 11 ++++++----- internal/kms/option_test.go | 8 ++++++++ internal/kms/options.go | 11 +++++++---- internal/kms/repository.go | 8 ++++++-- 5 files changed, 42 insertions(+), 11 deletions(-) diff --git a/internal/db/read_writer.go b/internal/db/read_writer.go index 4ceabd217d..e8461c4767 100644 --- a/internal/db/read_writer.go +++ b/internal/db/read_writer.go @@ -24,6 +24,21 @@ const ( DefaultLimit = 10000 ) +// OrderBy defines an enum type for declaring a column's order by criteria. +type OrderBy int + +const ( + // UnknownOrderBy would designate an unknown ordering of the column, which + // is the standard ordering for any select without an order by clause. + UnknownOrderBy = iota + + // AscendingOrderBy would designate ordering the column in ascending order. + AscendingOrderBy + + // DescendingOrderBy would designate ordering the column in decending order. + DescendingOrderBy +) + // Reader interface defines lookups/searching for resources type Reader interface { // LookupById will lookup a resource by its primary key id, which must be diff --git a/internal/kms/kms.go b/internal/kms/kms.go index d156198b86..7cce24027b 100644 --- a/internal/kms/kms.go +++ b/internal/kms/kms.go @@ -5,6 +5,7 @@ import ( "fmt" "sync" + "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/types/scope" "github.com/hashicorp/go-hclog" @@ -238,7 +239,7 @@ func (k *Kms) loadRoot(ctx context.Context, scopeId string, opt ...Option) (*mul if externalWrappers.root == nil { return nil, "", errors.New(errors.InvalidParameter, op, fmt.Sprintf("root key wrapper for scope %s is nil", scopeId)) } - rootKeyVersions, err := repo.ListRootKeyVersions(ctx, externalWrappers.root, rootKeyId, WithOrder("version desc")) + rootKeyVersions, err := repo.ListRootKeyVersions(ctx, externalWrappers.root, rootKeyId, WithOrderByVersion(db.DescendingOrderBy)) if err != nil { return nil, "", errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("error looking up root key versions for scope %s with key ID %s", scopeId, externalWrappers.root.KeyID()))) } @@ -325,13 +326,13 @@ func (k *Kms) loadDek(ctx context.Context, scopeId string, purpose KeyPurpose, r var keyVersions []DekVersion switch purpose { case KeyPurposeDatabase: - keyVersions, err = repo.ListDatabaseKeyVersions(ctx, rootWrapper, keyId, WithOrder("version desc")) + keyVersions, err = repo.ListDatabaseKeyVersions(ctx, rootWrapper, keyId, WithOrderByVersion(db.DescendingOrderBy)) case KeyPurposeOplog: - keyVersions, err = repo.ListOplogKeyVersions(ctx, rootWrapper, keyId, WithOrder("version desc")) + keyVersions, err = repo.ListOplogKeyVersions(ctx, rootWrapper, keyId, WithOrderByVersion(db.DescendingOrderBy)) case KeyPurposeTokens: - keyVersions, err = repo.ListTokenKeyVersions(ctx, rootWrapper, keyId, WithOrder("version desc")) + keyVersions, err = repo.ListTokenKeyVersions(ctx, rootWrapper, keyId, WithOrderByVersion(db.DescendingOrderBy)) case KeyPurposeSessions: - keyVersions, err = repo.ListSessionKeyVersions(ctx, rootWrapper, keyId, WithOrder("version desc")) + keyVersions, err = repo.ListSessionKeyVersions(ctx, rootWrapper, keyId, WithOrderByVersion(db.DescendingOrderBy)) default: return nil, errors.New(errors.InvalidParameter, op, "unknown or invalid DEK purpose specified") } diff --git a/internal/kms/option_test.go b/internal/kms/option_test.go index 7b8bf5ed74..c5e8e880c4 100644 --- a/internal/kms/option_test.go +++ b/internal/kms/option_test.go @@ -3,6 +3,7 @@ package kms import ( "testing" + "github.com/hashicorp/boundary/internal/db" "github.com/stretchr/testify/assert" ) @@ -27,4 +28,11 @@ func Test_GetOpts(t *testing.T) { testOpts.withLimit = 1 assert.Equal(opts, testOpts) }) + t.Run("WithOrderByVersion", func(t *testing.T) { + assert := assert.New(t) + opts := getOpts(WithOrderByVersion(db.DescendingOrderBy)) + testOpts := getDefaultOptions() + testOpts.withOrderByVersion = db.DescendingOrderBy + assert.Equal(opts, testOpts) + }) } diff --git a/internal/kms/options.go b/internal/kms/options.go index a078ae230d..4a19fb6e3d 100644 --- a/internal/kms/options.go +++ b/internal/kms/options.go @@ -1,6 +1,7 @@ package kms import ( + "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/go-hclog" wrapping "github.com/hashicorp/go-kms-wrapping" ) @@ -25,7 +26,7 @@ type options struct { withWorkerAuthWrapper wrapping.Wrapper withRecoveryWrapper wrapping.Wrapper withRepository *Repository - withOrder string + withOrderByVersion db.OrderBy withKeyId string } @@ -79,10 +80,12 @@ func WithRepository(repo *Repository) Option { } } -// WithOrder allows specifying an order for returned values -func WithOrder(order string) Option { +// WithOrderByVersion provides an option to specify ordering by the +// CreateTime field. +func WithOrderByVersion(orderBy db.OrderBy) Option { + const col = "version" return func(o *options) { - o.withOrder = order + o.withOrderByVersion = orderBy } } diff --git a/internal/kms/repository.go b/internal/kms/repository.go index d881424c61..00e405b32a 100644 --- a/internal/kms/repository.go +++ b/internal/kms/repository.go @@ -53,9 +53,13 @@ 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.withOrderByVersion { + case db.AscendingOrderBy: + dbOpts = append(dbOpts, db.WithOrder("version asc")) + case db.DescendingOrderBy: + dbOpts = append(dbOpts, db.WithOrder("version desc")) } + return r.reader.SearchWhere(ctx, resources, where, args, dbOpts...) }