From e09bf09a95db995538291c5accd0c6607ba420a4 Mon Sep 17 00:00:00 2001 From: Timothy Messier Date: Tue, 20 Feb 2024 21:36:05 +0000 Subject: [PATCH] refact(billing): Use sql functions instead of views These functions will ensure that UTC time is used for billing queries, eliminating the need for the repository to set the timezone. This also means the repository no longern needs a writer. --- internal/billing/query.go | 9 ++-- internal/billing/repository.go | 56 +++++++++--------------- internal/billing/repository_test.go | 15 +------ internal/billing/testing.go | 2 +- internal/daemon/controller/controller.go | 2 +- 5 files changed, 27 insertions(+), 57 deletions(-) diff --git a/internal/billing/query.go b/internal/billing/query.go index a792313c2..d74b18cb1 100644 --- a/internal/billing/query.go +++ b/internal/billing/query.go @@ -6,17 +6,14 @@ package billing const ( activeUsersLastTwoMonthsQuery = ` select * - from hcp_billing_monthly_active_users_last_2_months + from hcp_billing_monthly_active_users_last_2_months(); ` activeUsersWithStartTimeQuery = ` select * - from hcp_billing_monthly_active_users_all - where start_time >= @start_time + from hcp_billing_monthly_active_users_all(@start_time); ` activeUsersWithStartTimeAndEndTimeQuery = ` select * - from hcp_billing_monthly_active_users_all - where start_time >= @start_time - and end_time < @end_time + from hcp_billing_monthly_active_users_all(@start_time, @end_time); ` ) diff --git a/internal/billing/repository.go b/internal/billing/repository.go index bdff024c8..0c9a9a659 100644 --- a/internal/billing/repository.go +++ b/internal/billing/repository.go @@ -24,23 +24,19 @@ import ( // cumulative count up to the present date. type Repository struct { reader db.Reader - writer db.Writer } // NewRepository creates a new Repository. The returned repository is not safe for concurrent go // routines to access it. -func NewRepository(ctx context.Context, r db.Reader, w db.Writer) (*Repository, error) { +func NewRepository(ctx context.Context, r db.Reader) (*Repository, error) { const op = "billing.NewRepository" switch { case r == nil: return nil, errors.New(ctx, errors.InvalidParameter, op, "nil db reader") - case w == nil: - return nil, errors.New(ctx, errors.InvalidParameter, op, "nil db writer") } return &Repository{ reader: r, - writer: w, }, nil } @@ -81,40 +77,30 @@ func (r *Repository) MonthlyActiveUsers(ctx context.Context, opt ...Option) ([]A } var activeUsers []ActiveUsers - if _, err := r.writer.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, func(r db.Reader, w db.Writer) error { - _, err := w.Exec(ctx, `set timezone to 'utc'`, nil) - if err != nil { - return err + rows, err := r.reader.Query(ctx, query, args) + if err != nil { + return nil, err + } + defer rows.Close() + if err := rows.Err(); err != nil { + return nil, err + } + for rows.Next() { + var startTime time.Time + var endTime time.Time + var count uint64 + if err := rows.Scan(&startTime, &endTime, &count); err != nil { + return nil, err } - rows, err := r.Query(ctx, query, args) - if err != nil { - return err + // set start and end times to be in UTC + auUTC := ActiveUsers{ + ActiveUsersCount: count, + StartTime: startTime.UTC(), + EndTime: endTime.UTC(), } - defer rows.Close() - if err := rows.Err(); err != nil { - return err - } - for rows.Next() { - var start_time time.Time - var end_time time.Time - var count uint64 - if err := rows.Scan(&start_time, &end_time, &count); err != nil { - return err - } + activeUsers = append(activeUsers, auUTC) - // set start and end times to be in UTC - auUTC := ActiveUsers{ - ActiveUsersCount: count, - StartTime: start_time.UTC(), - EndTime: end_time.UTC(), - } - activeUsers = append(activeUsers, auUTC) - - } - return nil - }); err != nil { - return nil, errors.Wrap(ctx, err, op) } return activeUsers, nil diff --git a/internal/billing/repository_test.go b/internal/billing/repository_test.go index 6b370324b..4ff411446 100644 --- a/internal/billing/repository_test.go +++ b/internal/billing/repository_test.go @@ -35,39 +35,26 @@ func TestRepository_New(t *testing.T) { name: "valid", args: args{ r: rw, - w: rw, }, want: &Repository{ reader: rw, - writer: rw, }, }, { name: "nil-reader", args: args{ r: nil, - w: rw, }, want: nil, wantIsErr: errors.InvalidParameter, wantErrMsg: "billing.NewRepository: nil db reader: parameter violation: error #100", }, - { - name: "nil-writer", - args: args{ - r: rw, - w: nil, - }, - want: nil, - wantIsErr: errors.InvalidParameter, - wantErrMsg: "billing.NewRepository: nil db writer: parameter violation: error #100", - }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { assert := assert.New(t) - got, err := NewRepository(context.Background(), tt.args.r, tt.args.w) + got, err := NewRepository(context.Background(), tt.args.r) if tt.wantIsErr != 0 { assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "Unexpected error %s", err) assert.Equal(tt.wantErrMsg, err.Error()) diff --git a/internal/billing/testing.go b/internal/billing/testing.go index a657f8797..92b765a5e 100644 --- a/internal/billing/testing.go +++ b/internal/billing/testing.go @@ -88,7 +88,7 @@ func TestRepo(t testing.TB, conn *db.DB) *Repository { require := require.New(t) rw := db.New(conn) - repo, err := NewRepository(ctx, rw, rw) + repo, err := NewRepository(ctx, rw) require.NoError(err) return repo } diff --git a/internal/daemon/controller/controller.go b/internal/daemon/controller/controller.go index 94e60bf9b..5e0f90b58 100644 --- a/internal/daemon/controller/controller.go +++ b/internal/daemon/controller/controller.go @@ -451,7 +451,7 @@ func New(ctx context.Context, conf *Config) (*Controller, error) { return server.NewRepositoryStorage(ctx, dbase, dbase, c.kms) } c.BillingRepoFn = func() (*billing.Repository, error) { - return billing.NewRepository(ctx, dbase, dbase) + return billing.NewRepository(ctx, dbase) } // Check that credentials are available at startup, to avoid some harmless