Skip to content

Commit 34a202f

Browse files
committed
session: fix and test some return values
GetGroupID is given a session ID as a param and so should return ErrSessionNotFound if that session is not found. GetSessionIDs is given a groupID as a param and so should return ErrUnknownGroup if that group does not exist. This commit updates the kvstore and sql implementations to return the correct error values and also ensures that this is properly tested now.
1 parent 846dc68 commit 34a202f

File tree

3 files changed

+12
-4
lines changed

3 files changed

+12
-4
lines changed

session/kvdb_store.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -637,7 +637,7 @@ func (db *BoltStore) GetGroupID(_ context.Context, sessionID ID) (ID, error) {
637637
sessionIDBkt := idIndex.Bucket(sessionID[:])
638638
if sessionIDBkt == nil {
639639
return fmt.Errorf("%w: no index entry for session "+
640-
"ID: %x", ErrUnknownGroup, sessionID)
640+
"ID: %x", ErrSessionNotFound, sessionID)
641641
}
642642

643643
groupIDBytes := sessionIDBkt.Get(groupIDKey)
@@ -696,7 +696,7 @@ func getSessionIDs(sessionBkt *bbolt.Bucket, groupID ID) ([]ID, error) {
696696

697697
groupIDBkt := groupIndexBkt.Bucket(groupID[:])
698698
if groupIDBkt == nil {
699-
return nil, fmt.Errorf("no sessions for group ID %v",
699+
return nil, fmt.Errorf("%w: group ID %v", ErrUnknownGroup,
700700
groupID)
701701
}
702702

session/sql_store.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,7 @@ func (s *SQLStore) GetGroupID(ctx context.Context, sessionID ID) (ID, error) {
593593
// Get the session using the legacy Alias.
594594
sess, err := db.GetSessionByAlias(ctx, sessionID[:])
595595
if errors.Is(err, sql.ErrNoRows) {
596-
return ErrUnknownGroup
596+
return ErrSessionNotFound
597597
} else if err != nil {
598598
return err
599599
}

session/store_test.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,14 @@ func TestBasicSessionStore(t *testing.T) {
3232
_, err := db.GetSession(ctx, ID{1, 3, 4, 4})
3333
require.ErrorIs(t, err, ErrSessionNotFound)
3434

35+
// Do the same for other "Get" type methods to assert that the correct
36+
// errors are returned.
37+
_, err = db.GetGroupID(ctx, ID{1, 2, 3, 4})
38+
require.ErrorIs(t, err, ErrSessionNotFound)
39+
40+
_, err = db.GetSessionIDs(ctx, ID{1, 2, 3, 4})
41+
require.ErrorIs(t, err, ErrUnknownGroup)
42+
3543
// Reserve a session. This should succeed.
3644
s1, err := reserveSession(db, "session 1")
3745
require.NoError(t, err)
@@ -182,7 +190,7 @@ func TestBasicSessionStore(t *testing.T) {
182190
require.Empty(t, sessions)
183191

184192
_, err = db.GetGroupID(ctx, s4.ID)
185-
require.ErrorIs(t, err, ErrUnknownGroup)
193+
require.ErrorIs(t, err, ErrSessionNotFound)
186194

187195
// Only session 1 should remain in this group.
188196
sessIDs, err = db.GetSessionIDs(ctx, s4.GroupID)

0 commit comments

Comments
 (0)