From 0767b9f8eeae06508973a2f0354fa8738154acf2 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 a792313c27..d74b18cb17 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 bdff024c8f..0c9a9a6597 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 - } - defer rows.Close() - if err := rows.Err(); err != nil { - return err + // set start and end times to be in UTC + auUTC := ActiveUsers{ + ActiveUsersCount: count, + StartTime: startTime.UTC(), + EndTime: endTime.UTC(), } - 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 6b370324b7..4ff4114466 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 a657f87977..92b765a5e7 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 9414e8aa68..383e94303a 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