From 544ce112b59d9af8bae9594e2903ff283a9ff523 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Sun, 1 Mar 2026 11:44:22 -0500 Subject: [PATCH] Manual review --- user/manager.go | 15 +++++++-------- user/manager_postgres.go | 8 +++++--- user/manager_sqlite.go | 8 +++++--- user/manager_test.go | 2 +- user/types.go | 4 ++-- 5 files changed, 20 insertions(+), 17 deletions(-) diff --git a/user/manager.go b/user/manager.go index f991d296..fd2ac819 100644 --- a/user/manager.go +++ b/user/manager.go @@ -49,7 +49,7 @@ var ( type Manager struct { config *Config db *sql.DB - queries storeQueries + queries queries statsQueue map[string]*Stats // "Queue" to asynchronously write user stats to the database (UserID -> Stats) tokenQueue map[string]*TokenUpdate // "Queue" to asynchronously write token access stats to the database (Token ID -> TokenUpdate) mu sync.Mutex @@ -65,8 +65,6 @@ func initManager(manager *Manager) error { if manager.config.QueueWriterInterval.Seconds() <= 0 { manager.config.QueueWriterInterval = DefaultUserStatsQueueWriterInterval } - manager.statsQueue = make(map[string]*Stats) - manager.tokenQueue = make(map[string]*TokenUpdate) if err := manager.maybeProvisionUsersAccessAndTokens(); err != nil { return err } @@ -581,8 +579,7 @@ func (a *Manager) Authorize(user *User, topic string, perm Permission) error { read, write, found, err := a.authorizeTopicAccess(username, topic) if err != nil { return err - } - if !found { + } else if !found { return a.resolvePerms(a.config.DefaultAccess, perm) } return a.resolvePerms(NewPermission(read, write), perm) @@ -650,7 +647,7 @@ func (a *Manager) AllowReservation(username string, topic string) error { if (!AllowedUsername(username) && username != Everyone) || !AllowedTopic(topic) { return ErrInvalidArgument } - otherCount, err := a.OtherAccessCount(username, topic) + otherCount, err := a.otherAccessCount(username, topic) if err != nil { return err } @@ -853,8 +850,8 @@ func (a *Manager) ReservationOwner(topic string) (string, error) { return ownerUserID, nil } -// OtherAccessCount returns the number of access entries for the given topic that are not owned by the user -func (a *Manager) OtherAccessCount(username, topic string) (int, error) { +// otherAccessCount returns the number of access entries for the given topic that are not owned by the user +func (a *Manager) otherAccessCount(username, topic string) (int, error) { rows, err := a.db.Query(a.queries.selectOtherAccessCount, escapeUnderscore(topic), escapeUnderscore(topic), username) if err != nil { return 0, err @@ -919,6 +916,8 @@ func (a *Manager) createTokenTx(tx *sql.Tx, userID, token, label string, lastAcc return nil, err } if tokenCount > maxTokenCount { + // This pruning logic is done in two queries for efficiency. The SELECT above is a lookup + // on two indices, whereas the query below is a full table scan. if _, err := tx.Exec(a.queries.deleteExcessTokens, userID, userID, maxTokenCount); err != nil { return nil, err } diff --git a/user/manager_postgres.go b/user/manager_postgres.go index e396fbea..b7ef2d08 100644 --- a/user/manager_postgres.go +++ b/user/manager_postgres.go @@ -209,9 +209,11 @@ func NewPostgresManager(db *sql.DB, config *Config) (*Manager, error) { return nil, err } manager := &Manager{ - config: config, - db: db, - queries: storeQueries{ + config: config, + db: db, + statsQueue: make(map[string]*Stats), + tokenQueue: make(map[string]*TokenUpdate), + queries: queries{ // User queries selectUserByID: postgresSelectUserByIDQuery, selectUserByName: postgresSelectUserByNameQuery, diff --git a/user/manager_sqlite.go b/user/manager_sqlite.go index 7ca6e711..2d33087f 100644 --- a/user/manager_sqlite.go +++ b/user/manager_sqlite.go @@ -218,9 +218,11 @@ func NewSQLiteManager(filename, startupQueries string, config *Config) (*Manager return nil, err } manager := &Manager{ - config: config, - db: db, - queries: storeQueries{ + config: config, + db: db, + statsQueue: make(map[string]*Stats), + tokenQueue: make(map[string]*TokenUpdate), + queries: queries{ selectUserByID: sqliteSelectUserByIDQuery, selectUserByName: sqliteSelectUserByNameQuery, selectUserByToken: sqliteSelectUserByTokenQuery, diff --git a/user/manager_test.go b/user/manager_test.go index 64ea437a..dbdbd311 100644 --- a/user/manager_test.go +++ b/user/manager_test.go @@ -2402,7 +2402,7 @@ func TestStoreOtherAccessCount(t *testing.T) { require.Nil(t, manager.AddUser("ben", "benpass", RoleUser, false)) require.Nil(t, manager.AddReservation("ben", "mytopic", PermissionReadWrite)) - count, err := manager.OtherAccessCount("phil", "mytopic") + count, err := manager.otherAccessCount("phil", "mytopic") require.Nil(t, err) require.Equal(t, 2, count) // ben's owner entry + everyone entry }) diff --git a/user/types.go b/user/types.go index 58781203..e909ec78 100644 --- a/user/types.go +++ b/user/types.go @@ -275,8 +275,8 @@ var ( ErrProvisionedTokenChange = errors.New("cannot change or delete provisioned token") ) -// storeQueries holds the database-specific SQL queries -type storeQueries struct { +// queries holds the database-specific SQL queries +type queries struct { // User queries selectUserByID string selectUserByName string