Skip to content

Commit

Permalink
Lots of lint fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
tstirrat15 committed Sep 12, 2024
1 parent b1fc26d commit 9b9b291
Show file tree
Hide file tree
Showing 34 changed files with 266 additions and 78 deletions.
12 changes: 10 additions & 2 deletions internal/datastore/crdb/crdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/IBM/pgxpoolprometheus"
sq "github.com/Masterminds/squirrel"
"github.com/ccoveille/go-safecast"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgconn"
"github.com/jackc/pgx/v5/pgxpool"
Expand All @@ -28,6 +29,7 @@ import (
log "github.com/authzed/spicedb/internal/logging"
"github.com/authzed/spicedb/pkg/datastore"
"github.com/authzed/spicedb/pkg/datastore/options"
"github.com/authzed/spicedb/pkg/spiceerrors"
)

func init() {
Expand Down Expand Up @@ -420,8 +422,14 @@ func (cds *crdbDatastore) ReadyState(ctx context.Context) (datastore.ReadyState,
if writeMin > 0 {
writeMin--
}
writeTotal := uint32(cds.writePool.Stat().TotalConns())
readTotal := uint32(cds.readPool.Stat().TotalConns())
writeTotal, err := safecast.ToUint32(cds.writePool.Stat().TotalConns())
if err != nil {
return datastore.ReadyState{}, spiceerrors.MustBugf("could not cast writeTotal to uint32")
}
readTotal, err := safecast.ToUint32(cds.readPool.Stat().TotalConns())
if err != nil {
return datastore.ReadyState{}, spiceerrors.MustBugf("could not cast readTotal to uint32")
}
if writeTotal < writeMin || readTotal < readMin {
return datastore.ReadyState{
Message: fmt.Sprintf(
Expand Down
9 changes: 7 additions & 2 deletions internal/datastore/crdb/pool/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"sync"
"time"

"github.com/ccoveille/go-safecast"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgconn"
"github.com/jackc/pgx/v5/pgxpool"
Expand Down Expand Up @@ -141,12 +142,16 @@ func (p *RetryPool) ID() string {

// MaxConns returns the MaxConns configured on the underlying pool
func (p *RetryPool) MaxConns() uint32 {
return uint32(p.pool.Config().MaxConns)
// This should be non-negative
maxConns, _ := safecast.ToUint32(p.pool.Config().MaxConns)
return maxConns
}

// MinConns returns the MinConns configured on the underlying pool
func (p *RetryPool) MinConns() uint32 {
return uint32(p.pool.Config().MinConns)
// This should be non-negative
minConns, _ := safecast.ToUint32(p.pool.Config().MinConns)
return minConns
}

// ExecFunc is a replacement for pgxpool.Pool.Exec that allows resetting the
Expand Down
7 changes: 6 additions & 1 deletion internal/datastore/crdb/readwrite.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

sq "github.com/Masterminds/squirrel"
v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
"github.com/ccoveille/go-safecast"
"github.com/jackc/pgx/v5"
"github.com/jzelinskie/stringz"

Expand Down Expand Up @@ -430,7 +431,11 @@ func (rwt *crdbReadWriteTXN) DeleteRelationships(ctx context.Context, filter *v1
}

rwt.relCountChange -= modified.RowsAffected()
if delLimit > 0 && uint64(modified.RowsAffected()) == delLimit {
rowsAffected, err := safecast.ToUint64(modified.RowsAffected())
if err != nil {
return false, spiceerrors.MustBugf("could not cast RowsAffected to uint64")
}
if delLimit > 0 && rowsAffected == delLimit {
return true, nil
}

Expand Down
8 changes: 7 additions & 1 deletion internal/datastore/crdb/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import (
"slices"

"github.com/Masterminds/squirrel"
"github.com/ccoveille/go-safecast"
"github.com/jackc/pgx/v5"
"github.com/rs/zerolog/log"

pgxcommon "github.com/authzed/spicedb/internal/datastore/postgres/common"
"github.com/authzed/spicedb/pkg/datastore"
"github.com/authzed/spicedb/pkg/spiceerrors"
)

const (
Expand Down Expand Up @@ -118,7 +120,11 @@ func (cds *crdbDatastore) Statistics(ctx context.Context) (datastore.Stats, erro
return nil
}

estimatedRelCount = uint64(rowCount)
uintRowCount, err := safecast.ToUint64(rowCount)
if err != nil {
return spiceerrors.MustBugf("row count was negative")
}
estimatedRelCount = uintRowCount
return nil
}
}
Expand Down
9 changes: 8 additions & 1 deletion internal/datastore/mysql/gc.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import (
"time"

sq "github.com/Masterminds/squirrel"
"github.com/ccoveille/go-safecast"

"github.com/authzed/spicedb/internal/datastore/common"
"github.com/authzed/spicedb/internal/datastore/revisions"
log "github.com/authzed/spicedb/internal/logging"
"github.com/authzed/spicedb/pkg/datastore"
"github.com/authzed/spicedb/pkg/spiceerrors"
)

var _ common.GarbageCollector = (*Datastore)(nil)
Expand Down Expand Up @@ -64,7 +66,12 @@ func (mds *Datastore) TxIDBefore(ctx context.Context, before time.Time) (datasto
return datastore.NoRevision, nil
}

return revisions.NewForTransactionID(uint64(value.Int64)), nil
uintValue, err := safecast.ToUint64(value.Int64)
if err != nil {
return datastore.NoRevision, spiceerrors.MustBugf("value could not be cast to uint64")
}

return revisions.NewForTransactionID(uintValue), nil
}

// - implementation misses metrics
Expand Down
8 changes: 7 additions & 1 deletion internal/datastore/mysql/readwrite.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

sq "github.com/Masterminds/squirrel"
v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
"github.com/ccoveille/go-safecast"
"github.com/go-sql-driver/mysql"
"github.com/jzelinskie/stringz"

Expand Down Expand Up @@ -374,7 +375,12 @@ func (rwt *mysqlReadWriteTXN) DeleteRelationships(ctx context.Context, filter *v
return false, fmt.Errorf(errUnableToDeleteRelationships, err)
}

if delLimit > 0 && uint64(rowsAffected) == delLimit {
uintRowsAffected, err := safecast.ToUint64(rowsAffected)
if err != nil {
return false, spiceerrors.MustBugf("rowsAffected was negative")
}

if delLimit > 0 && uintRowsAffected == delLimit {
return true, nil
}

Expand Down
10 changes: 9 additions & 1 deletion internal/datastore/mysql/revisions.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@ import (
"fmt"
"time"

"github.com/ccoveille/go-safecast"

"github.com/authzed/spicedb/internal/datastore/revisions"
"github.com/authzed/spicedb/pkg/datastore"
"github.com/authzed/spicedb/pkg/spiceerrors"
)

var ParseRevisionString = revisions.RevisionParser(revisions.TransactionID)
Expand Down Expand Up @@ -175,5 +178,10 @@ func (mds *Datastore) createNewTransaction(ctx context.Context, tx *sql.Tx) (new
return 0, fmt.Errorf("createNewTransaction: failed to get last inserted id: %w", err)
}

return uint64(lastInsertID), nil
uintLastInsertID, err := safecast.ToUint64(lastInsertID)
if err != nil {
return 0, spiceerrors.MustBugf("lastInsertID was negative")
}

return uintLastInsertID, nil
}
9 changes: 8 additions & 1 deletion internal/datastore/mysql/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import (
"fmt"

"github.com/Masterminds/squirrel"
"github.com/ccoveille/go-safecast"

"github.com/authzed/spicedb/internal/datastore/common"
"github.com/authzed/spicedb/pkg/datastore"
"github.com/authzed/spicedb/pkg/spiceerrors"
)

const (
Expand Down Expand Up @@ -74,10 +76,15 @@ func (mds *Datastore) Statistics(ctx context.Context) (datastore.Stats, error) {
return datastore.Stats{}, fmt.Errorf("unable to load namespaces: %w", err)
}

uintCount, err := safecast.ToUint64(count.Int64)
if err != nil {
return datastore.Stats{}, spiceerrors.MustBugf("could not cast count to uint64")
}

return datastore.Stats{
UniqueID: uniqueID,
ObjectTypeStatistics: datastore.ComputeObjectTypeStats(nsDefs),
EstimatedRelationshipCount: uint64(count.Int64),
EstimatedRelationshipCount: uintCount,
}, nil
}

Expand Down
8 changes: 7 additions & 1 deletion internal/datastore/postgres/common/bulk.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ package common
import (
"context"

"github.com/ccoveille/go-safecast"
"github.com/jackc/pgx/v5"

"github.com/authzed/spicedb/pkg/datastore"
core "github.com/authzed/spicedb/pkg/proto/core/v1"
"github.com/authzed/spicedb/pkg/spiceerrors"
)

type tupleSourceAdapter struct {
Expand Down Expand Up @@ -74,5 +76,9 @@ func BulkLoad(
colNames: colNames,
}
copied, err := tx.CopyFrom(ctx, pgx.Identifier{tupleTableName}, colNames, adapter)
return uint64(copied), err
uintCopied, castErr := safecast.ToUint64(copied)
if castErr != nil {
return 0, spiceerrors.MustBugf("number copied was negative")
}
return uintCopied, err
}
1 change: 1 addition & 0 deletions internal/datastore/postgres/postgres_shared_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"errors"
"fmt"
"math"
"math/rand"
"strings"
"sync"
Expand Down
7 changes: 6 additions & 1 deletion internal/datastore/postgres/revisions.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,15 @@ func parseRevisionProto(revisionStr string) (datastore.Revision, error) {
}
}

xmax, err := safecast.ToUint64(xminInt + decoded.RelativeXmax)
if err != nil {
return datastore.NoRevision, spiceerrors.MustBugf("could not cast xmax to int64")
}

return postgresRevision{
snapshot: pgSnapshot{
xmin: decoded.Xmin,
xmax: uint64(xminInt + decoded.RelativeXmax),
xmax: xmax,
xipList: xips,
},
optionalTxID: xid8{Uint64: decoded.OptionalTxid, Valid: decoded.OptionalTxid != 0},
Expand Down
8 changes: 6 additions & 2 deletions internal/datastore/postgres/revisions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"
"time"

"github.com/ccoveille/go-safecast"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -50,7 +51,9 @@ func TestRevisionOrdering(t *testing.T) {
func TestRevisionSerDe(t *testing.T) {
maxSizeList := make([]uint64, 20)
for i := range maxSizeList {
maxSizeList[i] = maxInt - uint64(len(maxSizeList)) + uint64(i)
// i should be nonnegative
index, _ := safecast.ToUint64(i)
maxSizeList[i] = maxInt - uint64(len(maxSizeList)) + index
}

testCases := []struct {
Expand Down Expand Up @@ -85,7 +88,8 @@ func TestRevisionSerDe(t *testing.T) {
}

func TestTxIDTimestampAvailable(t *testing.T) {
testTimestamp := uint64(time.Now().Unix())
// Timestamps should be non-negative
testTimestamp, _ := safecast.ToUint64(time.Now().Unix())
snapshot := snap(0, 5, 1)
pgr := postgresRevision{snapshot: snapshot, optionalTxID: newXid8(1), optionalNanosTimestamp: testTimestamp}
receivedTimestamp, ok := pgr.OptionalNanosTimestamp()
Expand Down
5 changes: 4 additions & 1 deletion internal/datastore/postgres/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strconv"
"strings"

"github.com/ccoveille/go-safecast"
"github.com/jackc/pgx/v5/pgtype"
)

Expand Down Expand Up @@ -258,7 +259,9 @@ func (s pgSnapshot) markInProgress(txid uint64) pgSnapshot {
var numToDrop int
startingXipLen := len(newSnapshot.xipList)
for numToDrop = 0; numToDrop < startingXipLen; numToDrop++ {
if newSnapshot.xipList[startingXipLen-1-numToDrop] != newSnapshot.xmax-uint64(numToDrop)-1 {
// numToDrop should be nonnegative
uintNumToDrop, _ := safecast.ToUint64(numToDrop)
if newSnapshot.xipList[startingXipLen-1-numToDrop] != newSnapshot.xmax-uintNumToDrop-1 {
break
}
}
Expand Down
9 changes: 8 additions & 1 deletion internal/datastore/postgres/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ import (
"time"

sq "github.com/Masterminds/squirrel"
"github.com/ccoveille/go-safecast"
"github.com/jackc/pgx/v5"
"google.golang.org/protobuf/types/known/structpb"

"github.com/authzed/spicedb/internal/datastore/common"
pgxcommon "github.com/authzed/spicedb/internal/datastore/postgres/common"
"github.com/authzed/spicedb/pkg/datastore"
core "github.com/authzed/spicedb/pkg/proto/core/v1"
"github.com/authzed/spicedb/pkg/spiceerrors"
)

const (
Expand Down Expand Up @@ -204,10 +206,15 @@ func (pgd *pgDatastore) getNewRevisions(ctx context.Context, afterTX postgresRev
return fmt.Errorf("unable to decode new revision: %w", err)
}

nanosTimestamp, err := safecast.ToUint64(timestamp.UnixNano())
if err != nil {
return spiceerrors.MustBugf("could not cast timestamp to uint64")
}

ids = append(ids, postgresRevision{
snapshot: nextSnapshot.markComplete(nextXID.Uint64),
optionalTxID: nextXID,
optionalNanosTimestamp: uint64(timestamp.UnixNano()),
optionalNanosTimestamp: nanosTimestamp,
})
}
if rows.Err() != nil {
Expand Down
5 changes: 4 additions & 1 deletion internal/datastore/proxy/proxy_test/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
"github.com/ccoveille/go-safecast"
"github.com/stretchr/testify/mock"

"github.com/authzed/spicedb/pkg/datastore"
Expand Down Expand Up @@ -289,7 +290,9 @@ func (dm *MockReadWriteTransaction) DeleteNamespaces(_ context.Context, nsNames

func (dm *MockReadWriteTransaction) BulkLoad(_ context.Context, iter datastore.BulkWriteRelationshipSource) (uint64, error) {
args := dm.Called(iter)
return uint64(args.Int(0)), args.Error(1)
// We're assuming this is non-negative.
uintArg, _ := safecast.ToUint64(args.Int(0))
return uintArg, args.Error(1)
}

func (dm *MockReadWriteTransaction) ReadCaveatByName(_ context.Context, name string) (*core.CaveatDefinition, datastore.Revision, error) {
Expand Down
Loading

0 comments on commit 9b9b291

Please sign in to comment.