session: remove the filter fn in ListSessions

And instead let the caller pass in a list of States they are interested
in. This will make SQL queries much more efficient since we can index by
state.

Author: ellemouton

session/interface.go

@@ -161,8 +161,10 @@ type Store interface {
 	// GetSession fetches the session with the given key.
 	GetSession(key *btcec.PublicKey) (*Session, error)
 
-	// ListSessions returns all sessions currently known to the store.
-	ListSessions(filterFn func(s *Session) bool) ([]*Session, error)
+	// ListSessions returns all sessions currently known to the store that
+	// are in the given states. If no states are provided, all sessions are
+	// returned.
+	ListSessions(states ...State) ([]*Session, error)
 
 	// ListSessionsByType returns all sessions of the given type.
 	ListSessionsByType(t Type) ([]*Session, error)

session/kvdb_store.go

@@ -364,11 +364,24 @@ func (db *BoltStore) GetSession(key *btcec.PublicKey) (*Session, error) {
 	return session, nil
 }
 
-// ListSessions returns all sessions currently known to the store.
+// ListSessions returns all sessions currently known to the store that are in
+// the given states. If no states are provided, all sessions are returned.
 //
 // NOTE: this is part of the Store interface.
-func (db *BoltStore) ListSessions(filterFn func(s *Session) bool) ([]*Session, error) {
-	return db.listSessions(filterFn)
+func (db *BoltStore) ListSessions(states ...State) ([]*Session, error) {
+	return db.listSessions(func(s *Session) bool {
+		if len(states) == 0 {
+			return true
+		}
+
+		for _, state := range states {
+			if s.State == state {
+				return true
+			}
+		}
+
+		return false
+	})
 }
 
 // ListSessionsByType returns all sessions currently known to the store that

session/store_test.go

@@ -56,16 +56,8 @@ func TestBasicSessionStore(t *testing.T) {
 	require.NoError(t, db.CreateSession(s2))
 	require.NoError(t, db.CreateSession(s3))
 
-	// Check that all sessions are returned in ListSessions.
-	sessions, err := db.ListSessions(nil)
-	require.NoError(t, err)
-	require.Equal(t, 3, len(sessions))
-	assertEqualSessions(t, s1, sessions[0])
-	assertEqualSessions(t, s2, sessions[1])
-	assertEqualSessions(t, s3, sessions[2])
-
 	// Test the ListSessionsByType method.
-	sessions, err = db.ListSessionsByType(TypeMacaroonAdmin)
+	sessions, err := db.ListSessionsByType(TypeMacaroonAdmin)
 	require.NoError(t, err)
 	require.Equal(t, 2, len(sessions))
 	assertEqualSessions(t, s1, sessions[0])
@@ -115,9 +107,39 @@ func TestBasicSessionStore(t *testing.T) {
 
 	// Now revoke the session and assert that the state is revoked.
 	require.NoError(t, db.RevokeSession(s1.LocalPublicKey))
-	session1, err = db.GetSession(s1.LocalPublicKey)
+	s1, err = db.GetSession(s1.LocalPublicKey)
 	require.NoError(t, err)
-	require.Equal(t, session1.State, StateRevoked)
+	require.Equal(t, s1.State, StateRevoked)
+
+	// Test that ListSessions by certain states works.
+	sessions, err = db.ListSessions(StateRevoked)
+	require.NoError(t, err)
+	require.Equal(t, 1, len(sessions))
+	assertEqualSessions(t, s1, sessions[0])
+
+	sessions, err = db.ListSessions(StateCreated)
+	require.NoError(t, err)
+	require.Equal(t, 2, len(sessions))
+	assertEqualSessions(t, s2, sessions[0])
+	assertEqualSessions(t, s3, sessions[1])
+
+	sessions, err = db.ListSessions(StateCreated, StateRevoked)
+	require.NoError(t, err)
+	require.Equal(t, 3, len(sessions))
+	assertEqualSessions(t, s1, sessions[0])
+	assertEqualSessions(t, s2, sessions[1])
+	assertEqualSessions(t, s3, sessions[2])
+
+	sessions, err = db.ListSessions()
+	require.NoError(t, err)
+	require.Equal(t, 3, len(sessions))
+	assertEqualSessions(t, s1, sessions[0])
+	assertEqualSessions(t, s2, sessions[1])
+	assertEqualSessions(t, s3, sessions[2])
+
+	sessions, err = db.ListSessions(StateInUse)
+	require.NoError(t, err)
+	require.Empty(t, sessions)
 }
 
 // TestLinkingSessions tests that session linking works as expected.

session_rpcserver.go

@@ -101,7 +101,7 @@ func newSessionRPCServer(cfg *sessionRpcServerConfig) (*sessionRpcServer,
 // requests. This includes resuming all non-revoked sessions.
 func (s *sessionRpcServer) start(ctx context.Context) error {
 	// Start up all previously created sessions.
-	sessions, err := s.cfg.db.ListSessions(nil)
+	sessions, err := s.cfg.db.ListSessions()
 	if err != nil {
 		return fmt.Errorf("error listing sessions: %v", err)
 	}
@@ -536,7 +536,7 @@ func (s *sessionRpcServer) resumeSession(ctx context.Context,
 func (s *sessionRpcServer) ListSessions(_ context.Context,
 	_ *litrpc.ListSessionsRequest) (*litrpc.ListSessionsResponse, error) {
 
-	sessions, err := s.cfg.db.ListSessions(nil)
+	sessions, err := s.cfg.db.ListSessions()
 	if err != nil {
 		return nil, fmt.Errorf("error fetching sessions: %v", err)
 	}