From 15cd98be222ef1a1db4b0dfa53816287ab823eca Mon Sep 17 00:00:00 2001 From: David Zhao Date: Mon, 27 Dec 2021 23:32:29 -0800 Subject: [PATCH] Enable Room.List to filter by specific names (#290) --- go.mod | 2 +- go.sum | 4 +-- pkg/service/interfaces.go | 4 ++- pkg/service/localroomstore.go | 7 ++-- pkg/service/redisroomstore.go | 23 ++++++++++--- pkg/service/roommanager.go | 2 +- pkg/service/roomservice.go | 8 +++-- pkg/service/servicefakes/fake_room_store.go | 23 ++++++++----- pkg/service/servicefakes/fake_roroom_store.go | 23 ++++++++----- test/integration_helpers.go | 16 ++++++++-- test/multinode_test.go | 15 +++++++-- test/scenarios.go | 32 +++++++++++++++++-- test/singlenode_test.go | 12 ++++++- 13 files changed, 134 insertions(+), 37 deletions(-) diff --git a/go.mod b/go.mod index 3663352a3..9f2601be1 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/google/wire v0.5.0 github.com/gorilla/websocket v1.4.2 github.com/hashicorp/golang-lru v0.5.4 - github.com/livekit/protocol v0.11.3 + github.com/livekit/protocol v0.11.4 github.com/magefile/mage v1.11.0 github.com/maxbrunsfeld/counterfeiter/v6 v6.3.0 github.com/mitchellh/go-homedir v1.1.0 diff --git a/go.sum b/go.sum index fda65bff3..bb915efd9 100644 --- a/go.sum +++ b/go.sum @@ -132,8 +132,8 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/lithammer/shortuuid/v3 v3.0.6 h1:pr15YQyvhiSX/qPxncFtqk+v4xLEpOZObbsY/mKrcvA= github.com/lithammer/shortuuid/v3 v3.0.6/go.mod h1:vMk8ke37EmiewwolSO1NLW8vP4ZaKlRuDIi8tWWmAts= -github.com/livekit/protocol v0.11.3 h1:Al2oOrRwFNmgpw7dUvvc0s+oju9DoRUWi7g7GwrDiZc= -github.com/livekit/protocol v0.11.3/go.mod h1:YoHW9YbWbPnuVsgwBB4hAINKT+V68jmfh9zXBSSn6Wg= +github.com/livekit/protocol v0.11.4 h1:p4ZA/OW+Wuc3q48DdeSFUAFaTHmqz62/C/LXM3D0/Z4= +github.com/livekit/protocol v0.11.4/go.mod h1:YoHW9YbWbPnuVsgwBB4hAINKT+V68jmfh9zXBSSn6Wg= github.com/magefile/mage v1.11.0 h1:C/55Ywp9BpgVVclD3lRnSYCwXTYxmSppIgLeDYlNuls= github.com/magefile/mage v1.11.0/go.mod h1:z5UZb/iS3GoOSn0JgWuiw7dxlurVYTu+/jHXqQg881A= github.com/mattn/go-runewidth v0.0.9 h1:Lm995f3rfxdpd6TSmuVCHVb/QhupuXlYr8sCI/QdE+0= diff --git a/pkg/service/interfaces.go b/pkg/service/interfaces.go index b4b3f99cc..04cefe022 100644 --- a/pkg/service/interfaces.go +++ b/pkg/service/interfaces.go @@ -29,7 +29,9 @@ type RoomStore interface { //counterfeiter:generate . RORoomStore type RORoomStore interface { LoadRoom(ctx context.Context, name string) (*livekit.Room, error) - ListRooms(ctx context.Context) ([]*livekit.Room, error) + // ListRooms returns currently active rooms. if names is not nil, it'll filter and return + // only rooms that match + ListRooms(ctx context.Context, names []string) ([]*livekit.Room, error) LoadParticipant(ctx context.Context, roomName, identity string) (*livekit.ParticipantInfo, error) ListParticipants(ctx context.Context, roomName string) ([]*livekit.ParticipantInfo, error) diff --git a/pkg/service/localroomstore.go b/pkg/service/localroomstore.go index b7c96b20e..eaf51bcee 100644 --- a/pkg/service/localroomstore.go +++ b/pkg/service/localroomstore.go @@ -6,6 +6,7 @@ import ( "time" "github.com/livekit/protocol/livekit" + "github.com/thoas/go-funk" ) // encapsulates CRUD operations for room settings @@ -47,12 +48,14 @@ func (p *LocalRoomStore) LoadRoom(_ context.Context, name string) (*livekit.Room return room, nil } -func (p *LocalRoomStore) ListRooms(_ context.Context) ([]*livekit.Room, error) { +func (p *LocalRoomStore) ListRooms(_ context.Context, names []string) ([]*livekit.Room, error) { p.lock.RLock() defer p.lock.RUnlock() rooms := make([]*livekit.Room, 0, len(p.rooms)) for _, r := range p.rooms { - rooms = append(rooms, r) + if names == nil || funk.Contains(names, r.Name) { + rooms = append(rooms, r) + } } return rooms, nil } diff --git a/pkg/service/redisroomstore.go b/pkg/service/redisroomstore.go index 904ca39c3..9ee9a52f3 100644 --- a/pkg/service/redisroomstore.go +++ b/pkg/service/redisroomstore.go @@ -72,10 +72,25 @@ func (p *RedisRoomStore) LoadRoom(_ context.Context, name string) (*livekit.Room return &room, nil } -func (p *RedisRoomStore) ListRooms(_ context.Context) ([]*livekit.Room, error) { - items, err := p.rc.HVals(p.ctx, RoomsKey).Result() - if err != nil && err != redis.Nil { - return nil, errors.Wrap(err, "could not get rooms") +func (p *RedisRoomStore) ListRooms(_ context.Context, names []string) ([]*livekit.Room, error) { + var items []string + var err error + if names == nil { + items, err = p.rc.HVals(p.ctx, RoomsKey).Result() + if err != nil && err != redis.Nil { + return nil, errors.Wrap(err, "could not get rooms") + } + } else { + var results []interface{} + results, err = p.rc.HMGet(p.ctx, RoomsKey, names...).Result() + if err != nil && err != redis.Nil { + return nil, errors.Wrap(err, "could not get rooms by names") + } + for _, r := range results { + if item, ok := r.(string); ok { + items = append(items, item) + } + } } rooms := make([]*livekit.Room, 0, len(items)) diff --git a/pkg/service/roommanager.go b/pkg/service/roommanager.go index bca81e9d1..0d10e66e0 100644 --- a/pkg/service/roommanager.go +++ b/pkg/service/roommanager.go @@ -105,7 +105,7 @@ func (r *RoomManager) DeleteRoom(ctx context.Context, roomName string) error { func (r *RoomManager) CleanupRooms() error { // cleanup rooms that have been left for over a day ctx := context.Background() - rooms, err := r.roomStore.ListRooms(ctx) + rooms, err := r.roomStore.ListRooms(ctx, nil) if err != nil { return err } diff --git a/pkg/service/roomservice.go b/pkg/service/roomservice.go index 15d37257a..dbf3908f7 100644 --- a/pkg/service/roomservice.go +++ b/pkg/service/roomservice.go @@ -40,13 +40,17 @@ func (s *RoomService) CreateRoom(ctx context.Context, req *livekit.CreateRoomReq return } -func (s *RoomService) ListRooms(ctx context.Context, _ *livekit.ListRoomsRequest) (res *livekit.ListRoomsResponse, err error) { +func (s *RoomService) ListRooms(ctx context.Context, req *livekit.ListRoomsRequest) (res *livekit.ListRoomsResponse, err error) { err = EnsureListPermission(ctx) if err != nil { return nil, twirpAuthError(err) } - rooms, err := s.roomStore.ListRooms(ctx) + var names []string + if len(req.Names) > 0 { + names = req.Names + } + rooms, err := s.roomStore.ListRooms(ctx, names) if err != nil { // TODO: translate error codes to twirp return diff --git a/pkg/service/servicefakes/fake_room_store.go b/pkg/service/servicefakes/fake_room_store.go index d6c46dab7..f9eedaac1 100644 --- a/pkg/service/servicefakes/fake_room_store.go +++ b/pkg/service/servicefakes/fake_room_store.go @@ -50,10 +50,11 @@ type FakeRoomStore struct { result1 []*livekit.ParticipantInfo result2 error } - ListRoomsStub func(context.Context) ([]*livekit.Room, error) + ListRoomsStub func(context.Context, []string) ([]*livekit.Room, error) listRoomsMutex sync.RWMutex listRoomsArgsForCall []struct { arg1 context.Context + arg2 []string } listRoomsReturns struct { result1 []*livekit.Room @@ -339,18 +340,24 @@ func (fake *FakeRoomStore) ListParticipantsReturnsOnCall(i int, result1 []*livek }{result1, result2} } -func (fake *FakeRoomStore) ListRooms(arg1 context.Context) ([]*livekit.Room, error) { +func (fake *FakeRoomStore) ListRooms(arg1 context.Context, arg2 []string) ([]*livekit.Room, error) { + var arg2Copy []string + if arg2 != nil { + arg2Copy = make([]string, len(arg2)) + copy(arg2Copy, arg2) + } fake.listRoomsMutex.Lock() ret, specificReturn := fake.listRoomsReturnsOnCall[len(fake.listRoomsArgsForCall)] fake.listRoomsArgsForCall = append(fake.listRoomsArgsForCall, struct { arg1 context.Context - }{arg1}) + arg2 []string + }{arg1, arg2Copy}) stub := fake.ListRoomsStub fakeReturns := fake.listRoomsReturns - fake.recordInvocation("ListRooms", []interface{}{arg1}) + fake.recordInvocation("ListRooms", []interface{}{arg1, arg2Copy}) fake.listRoomsMutex.Unlock() if stub != nil { - return stub(arg1) + return stub(arg1, arg2) } if specificReturn { return ret.result1, ret.result2 @@ -364,17 +371,17 @@ func (fake *FakeRoomStore) ListRoomsCallCount() int { return len(fake.listRoomsArgsForCall) } -func (fake *FakeRoomStore) ListRoomsCalls(stub func(context.Context) ([]*livekit.Room, error)) { +func (fake *FakeRoomStore) ListRoomsCalls(stub func(context.Context, []string) ([]*livekit.Room, error)) { fake.listRoomsMutex.Lock() defer fake.listRoomsMutex.Unlock() fake.ListRoomsStub = stub } -func (fake *FakeRoomStore) ListRoomsArgsForCall(i int) context.Context { +func (fake *FakeRoomStore) ListRoomsArgsForCall(i int) (context.Context, []string) { fake.listRoomsMutex.RLock() defer fake.listRoomsMutex.RUnlock() argsForCall := fake.listRoomsArgsForCall[i] - return argsForCall.arg1 + return argsForCall.arg1, argsForCall.arg2 } func (fake *FakeRoomStore) ListRoomsReturns(result1 []*livekit.Room, result2 error) { diff --git a/pkg/service/servicefakes/fake_roroom_store.go b/pkg/service/servicefakes/fake_roroom_store.go index 2469e90d6..4ccd13a7b 100644 --- a/pkg/service/servicefakes/fake_roroom_store.go +++ b/pkg/service/servicefakes/fake_roroom_store.go @@ -24,10 +24,11 @@ type FakeRORoomStore struct { result1 []*livekit.ParticipantInfo result2 error } - ListRoomsStub func(context.Context) ([]*livekit.Room, error) + ListRoomsStub func(context.Context, []string) ([]*livekit.Room, error) listRoomsMutex sync.RWMutex listRoomsArgsForCall []struct { arg1 context.Context + arg2 []string } listRoomsReturns struct { result1 []*livekit.Room @@ -135,18 +136,24 @@ func (fake *FakeRORoomStore) ListParticipantsReturnsOnCall(i int, result1 []*liv }{result1, result2} } -func (fake *FakeRORoomStore) ListRooms(arg1 context.Context) ([]*livekit.Room, error) { +func (fake *FakeRORoomStore) ListRooms(arg1 context.Context, arg2 []string) ([]*livekit.Room, error) { + var arg2Copy []string + if arg2 != nil { + arg2Copy = make([]string, len(arg2)) + copy(arg2Copy, arg2) + } fake.listRoomsMutex.Lock() ret, specificReturn := fake.listRoomsReturnsOnCall[len(fake.listRoomsArgsForCall)] fake.listRoomsArgsForCall = append(fake.listRoomsArgsForCall, struct { arg1 context.Context - }{arg1}) + arg2 []string + }{arg1, arg2Copy}) stub := fake.ListRoomsStub fakeReturns := fake.listRoomsReturns - fake.recordInvocation("ListRooms", []interface{}{arg1}) + fake.recordInvocation("ListRooms", []interface{}{arg1, arg2Copy}) fake.listRoomsMutex.Unlock() if stub != nil { - return stub(arg1) + return stub(arg1, arg2) } if specificReturn { return ret.result1, ret.result2 @@ -160,17 +167,17 @@ func (fake *FakeRORoomStore) ListRoomsCallCount() int { return len(fake.listRoomsArgsForCall) } -func (fake *FakeRORoomStore) ListRoomsCalls(stub func(context.Context) ([]*livekit.Room, error)) { +func (fake *FakeRORoomStore) ListRoomsCalls(stub func(context.Context, []string) ([]*livekit.Room, error)) { fake.listRoomsMutex.Lock() defer fake.listRoomsMutex.Unlock() fake.ListRoomsStub = stub } -func (fake *FakeRORoomStore) ListRoomsArgsForCall(i int) context.Context { +func (fake *FakeRORoomStore) ListRoomsArgsForCall(i int) (context.Context, []string) { fake.listRoomsMutex.RLock() defer fake.listRoomsMutex.RUnlock() argsForCall := fake.listRoomsArgsForCall[i] - return argsForCall.arg1 + return argsForCall.arg1, argsForCall.arg2 } func (fake *FakeRORoomStore) ListRoomsReturns(result1 []*livekit.Room, result2 error) { diff --git a/test/integration_helpers.go b/test/integration_helpers.go index 1fd93430e..382de0d8c 100644 --- a/test/integration_helpers.go +++ b/test/integration_helpers.go @@ -58,7 +58,7 @@ func setupSingleNodeTest(name string, roomName string) (*service.LivekitServer, waitForServerToStart(s) // create test room - _, err := roomClient.CreateRoom(contextWithCreateRoomToken(), &livekit.CreateRoomRequest{Name: roomName}) + _, err := roomClient.CreateRoom(contextWithToken(createRoomToken()), &livekit.CreateRoomRequest{Name: roomName}) if err != nil { panic(err) } @@ -86,9 +86,9 @@ func setupMultiNodeTest(name string) (*service.LivekitServer, *service.LivekitSe } } -func contextWithCreateRoomToken() context.Context { +func contextWithToken(token string) context.Context { header := make(http.Header) - testclient.SetAuthorizationToken(header, createRoomToken()) + testclient.SetAuthorizationToken(header, token) tctx, err := twirp.WithHTTPRequestHeaders(context.Background(), header) if err != nil { panic(err) @@ -258,6 +258,16 @@ func createRoomToken() string { return t } +func listRoomToken() string { + at := auth.NewAccessToken(testApiKey, testApiSecret). + AddGrant(&auth.VideoGrant{RoomList: true}) + t, err := at.ToJWT() + if err != nil { + panic(err) + } + return t +} + func stopWriters(writers ...*testclient.TrackWriter) { for _, w := range writers { w.Stop() diff --git a/test/multinode_test.go b/test/multinode_test.go index 1be2f8c83..eb22341cb 100644 --- a/test/multinode_test.go +++ b/test/multinode_test.go @@ -20,7 +20,7 @@ func TestMultiNodeRouting(t *testing.T) { defer finish() // creating room on node 1 - _, err := roomClient.CreateRoom(contextWithCreateRoomToken(), &livekit.CreateRoomRequest{ + _, err := roomClient.CreateRoom(contextWithToken(createRoomToken()), &livekit.CreateRoomRequest{ Name: testRoom, }) require.NoError(t, err) @@ -103,7 +103,7 @@ func TestMultinodeReconnectAfterNodeShutdown(t *testing.T) { defer finish() // creating room on node 1 - _, err := roomClient.CreateRoom(contextWithCreateRoomToken(), &livekit.CreateRoomRequest{ + _, err := roomClient.CreateRoom(contextWithToken(createRoomToken()), &livekit.CreateRoomRequest{ Name: testRoom, NodeId: s2.Node().Id, }) @@ -136,3 +136,14 @@ func TestMultinodeDataPublishing(t *testing.T) { scenarioDataPublish(t) } + +func TestMultiNodeRoomList(t *testing.T) { + if testing.Short() { + t.SkipNow() + return + } + _, _, finish := setupMultiNodeTest("TestMultiNodeRoomList") + defer finish() + + roomServiceListRoom(t) +} diff --git a/test/scenarios.go b/test/scenarios.go index 70c31ed52..6960a4054 100644 --- a/test/scenarios.go +++ b/test/scenarios.go @@ -141,8 +141,6 @@ func scenarioDataPublish(t *testing.T) { }) } -// websocket reconnects - func publishTracksForClients(t *testing.T, clients ...*testclient.RTCClient) []*testclient.TrackWriter { logger.Infow("publishing tracks for clients") var writers []*testclient.TrackWriter @@ -158,3 +156,33 @@ func publishTracksForClients(t *testing.T, clients ...*testclient.RTCClient) []* } return writers } + +// Room service tests + +func roomServiceListRoom(t *testing.T) { + createCtx := contextWithToken(createRoomToken()) + listCtx := contextWithToken(listRoomToken()) + // create rooms + _, err := roomClient.CreateRoom(createCtx, &livekit.CreateRoomRequest{ + Name: testRoom, + }) + require.NoError(t, err) + _, err = roomClient.CreateRoom(contextWithToken(createRoomToken()), &livekit.CreateRoomRequest{ + Name: "yourroom", + }) + require.NoError(t, err) + + t.Run("list all rooms", func(t *testing.T) { + res, err := roomClient.ListRooms(listCtx, &livekit.ListRoomsRequest{}) + require.NoError(t, err) + require.Len(t, res.Rooms, 2) + }) + t.Run("list specific rooms", func(t *testing.T) { + res, err := roomClient.ListRooms(listCtx, &livekit.ListRoomsRequest{ + Names: []string{"yourroom"}, + }) + require.NoError(t, err) + require.Len(t, res.Rooms, 1) + require.Equal(t, "yourroom", res.Rooms[0].Name) + }) +} diff --git a/test/singlenode_test.go b/test/singlenode_test.go index 591132261..526713d61 100644 --- a/test/singlenode_test.go +++ b/test/singlenode_test.go @@ -291,5 +291,15 @@ func Test_RenegotiationWithDifferentCodecs(t *testing.T) { if !success { t.FailNow() } - +} + +func TestSingleNodeRoomList(t *testing.T) { + if testing.Short() { + t.SkipNow() + return + } + _, finish := setupSingleNodeTest("TestSingleNodeRoomList", testRoom) + defer finish() + + roomServiceListRoom(t) }