From e624604453d76ad7b0ed5e46f0bfd644630cb220 Mon Sep 17 00:00:00 2001 From: Raja Subramanian Date: Thu, 21 Jul 2022 14:49:41 +0530 Subject: [PATCH] Revert "ListRooms using `sid` (#842)" (#845) This reverts commit f2e1e67e5831276d8896403aeee3b027817fff2e. --- go.mod | 2 +- go.sum | 4 +-- pkg/service/interfaces.go | 4 +-- pkg/service/localstore.go | 4 +-- pkg/service/redisstore.go | 29 +++++++++++------ pkg/service/roommanager.go | 2 +- pkg/service/roomservice.go | 6 +--- pkg/service/servicefakes/fake_object_store.go | 23 +++++--------- .../servicefakes/fake_service_store.go | 23 +++++--------- test/scenarios.go | 31 ++----------------- 10 files changed, 48 insertions(+), 80 deletions(-) diff --git a/go.mod b/go.mod index 26a3e6a59..08f66b8d7 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,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.13.5-0.20220721052645-5eea37b737f6 + github.com/livekit/protocol v0.13.5-0.20220721030958-86da2252193b github.com/livekit/rtcscore-go v0.0.0-20220524203225-dfd1ba40744a github.com/mackerelio/go-osstat v0.2.1 github.com/magefile/mage v1.13.0 diff --git a/go.sum b/go.sum index 4934e35ec..1e4b1179c 100644 --- a/go.sum +++ b/go.sum @@ -235,8 +235,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/lithammer/shortuuid/v3 v3.0.7 h1:trX0KTHy4Pbwo/6ia8fscyHoGA+mf1jWbPJVuvyJQQ8= github.com/lithammer/shortuuid/v3 v3.0.7/go.mod h1:vMk8ke37EmiewwolSO1NLW8vP4ZaKlRuDIi8tWWmAts= -github.com/livekit/protocol v0.13.5-0.20220721052645-5eea37b737f6 h1:+8Uw5Ron7LU1nXMn7lfwB91G0fGpjqMlRHvxdyE/JUk= -github.com/livekit/protocol v0.13.5-0.20220721052645-5eea37b737f6/go.mod h1:Qd/Dn4BkJfZQy/IjtEeUOGXARrR7l09WDkg5SY8thkw= +github.com/livekit/protocol v0.13.5-0.20220721030958-86da2252193b h1:7p5M5WoTFDyIvxcB+r2aqMYKU8tFYf8MA52vXY15naI= +github.com/livekit/protocol v0.13.5-0.20220721030958-86da2252193b/go.mod h1:Qd/Dn4BkJfZQy/IjtEeUOGXARrR7l09WDkg5SY8thkw= github.com/livekit/rtcscore-go v0.0.0-20220524203225-dfd1ba40744a h1:cENjhGfslLSDV07gt8ASy47Wd12Q0kBS7hsdunyQ62I= github.com/livekit/rtcscore-go v0.0.0-20220524203225-dfd1ba40744a/go.mod h1:116ych8UaEs9vfIE8n6iZCZ30iagUFTls0vRmC+Ix5U= github.com/mackerelio/go-osstat v0.2.1 h1:5AeAcBEutEErAOlDz6WCkEvm6AKYgHTUQrfwm5RbeQc= diff --git a/pkg/service/interfaces.go b/pkg/service/interfaces.go index 47146215f..b36e254dc 100644 --- a/pkg/service/interfaces.go +++ b/pkg/service/interfaces.go @@ -29,9 +29,9 @@ type ObjectStore interface { //counterfeiter:generate . ServiceStore type ServiceStore interface { LoadRoom(ctx context.Context, name livekit.RoomName) (*livekit.Room, error) - // ListRooms returns currently active rooms. if names and/or sids is not nil, it'll filter and return + // ListRooms returns currently active rooms. if names is not nil, it'll filter and return // only rooms that match - ListRooms(ctx context.Context, names []livekit.RoomName, sids []livekit.RoomID) ([]*livekit.Room, error) + ListRooms(ctx context.Context, names []livekit.RoomName) ([]*livekit.Room, error) LoadParticipant(ctx context.Context, roomName livekit.RoomName, identity livekit.ParticipantIdentity) (*livekit.ParticipantInfo, error) ListParticipants(ctx context.Context, roomName livekit.RoomName) ([]*livekit.ParticipantInfo, error) diff --git a/pkg/service/localstore.go b/pkg/service/localstore.go index 463e82172..9f9e86e21 100644 --- a/pkg/service/localstore.go +++ b/pkg/service/localstore.go @@ -50,12 +50,12 @@ func (s *LocalStore) LoadRoom(_ context.Context, name livekit.RoomName) (*liveki return room, nil } -func (s *LocalStore) ListRooms(_ context.Context, names []livekit.RoomName, sids []livekit.RoomID) ([]*livekit.Room, error) { +func (s *LocalStore) ListRooms(_ context.Context, names []livekit.RoomName) ([]*livekit.Room, error) { s.lock.RLock() defer s.lock.RUnlock() rooms := make([]*livekit.Room, 0, len(s.rooms)) for _, r := range s.rooms { - if (names == nil && sids == nil) || funk.Contains(names, livekit.RoomName(r.Name)) || funk.Contains(sids, livekit.RoomID(r.Sid)) { + if names == nil || funk.Contains(names, livekit.RoomName(r.Name)) { rooms = append(rooms, r) } } diff --git a/pkg/service/redisstore.go b/pkg/service/redisstore.go index fc851094d..eab293abe 100644 --- a/pkg/service/redisstore.go +++ b/pkg/service/redisstore.go @@ -6,7 +6,6 @@ import ( "github.com/go-redis/redis/v8" "github.com/pkg/errors" - "github.com/thoas/go-funk" "google.golang.org/protobuf/proto" "github.com/livekit/protocol/livekit" @@ -77,25 +76,37 @@ func (s *RedisStore) LoadRoom(_ context.Context, name livekit.RoomName) (*liveki return &room, nil } -func (s *RedisStore) ListRooms(_ context.Context, names []livekit.RoomName, sids []livekit.RoomID) ([]*livekit.Room, error) { +func (s *RedisStore) ListRooms(_ context.Context, names []livekit.RoomName) ([]*livekit.Room, error) { var items []string var err error - items, err = s.rc.HVals(s.ctx, RoomsKey).Result() - if err != nil && err != redis.Nil { - return nil, errors.Wrap(err, "could not get rooms") + if names == nil { + items, err = s.rc.HVals(s.ctx, RoomsKey).Result() + if err != nil && err != redis.Nil { + return nil, errors.Wrap(err, "could not get rooms") + } + } else { + roomNames := livekit.RoomNamesAsStrings(names) + var results []interface{} + results, err = s.rc.HMGet(s.ctx, RoomsKey, roomNames...).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)) + for _, item := range items { room := livekit.Room{} err := proto.Unmarshal([]byte(item), &room) if err != nil { return nil, err } - - if (names == nil && sids == nil) || funk.Contains(names, livekit.RoomName(room.Name)) || funk.Contains(sids, livekit.RoomID(room.Sid)) { - rooms = append(rooms, &room) - } + rooms = append(rooms, &room) } return rooms, nil } diff --git a/pkg/service/roommanager.go b/pkg/service/roommanager.go index c05b529b7..9ec95bd2e 100644 --- a/pkg/service/roommanager.go +++ b/pkg/service/roommanager.go @@ -114,7 +114,7 @@ func (r *RoomManager) DeleteRoom(ctx context.Context, roomName livekit.RoomName) func (r *RoomManager) CleanupRooms() error { // cleanup rooms that have been left for over a day ctx := context.Background() - rooms, err := r.roomStore.ListRooms(ctx, nil, nil) + 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 c32ec28d9..d5bad1c24 100644 --- a/pkg/service/roomservice.go +++ b/pkg/service/roomservice.go @@ -61,11 +61,7 @@ func (s *RoomService) ListRooms(ctx context.Context, req *livekit.ListRoomsReque if len(req.Names) > 0 { names = livekit.StringsAsRoomNames(req.Names) } - var sids []livekit.RoomID - if len(req.RoomSids) > 0 { - sids = livekit.StringsAsRoomIDs(req.RoomSids) - } - rooms, err := s.roomStore.ListRooms(ctx, names, sids) + rooms, err := s.roomStore.ListRooms(ctx, names) if err != nil { // TODO: translate error codes to twirp return diff --git a/pkg/service/servicefakes/fake_object_store.go b/pkg/service/servicefakes/fake_object_store.go index 302eea499..39ff2a54d 100644 --- a/pkg/service/servicefakes/fake_object_store.go +++ b/pkg/service/servicefakes/fake_object_store.go @@ -76,12 +76,11 @@ type FakeObjectStore struct { result1 []*livekit.ParticipantInfo result2 error } - ListRoomsStub func(context.Context, []livekit.RoomName, []livekit.RoomID) ([]*livekit.Room, error) + ListRoomsStub func(context.Context, []livekit.RoomName) ([]*livekit.Room, error) listRoomsMutex sync.RWMutex listRoomsArgsForCall []struct { arg1 context.Context arg2 []livekit.RoomName - arg3 []livekit.RoomID } listRoomsReturns struct { result1 []*livekit.Room @@ -532,30 +531,24 @@ func (fake *FakeObjectStore) ListParticipantsReturnsOnCall(i int, result1 []*liv }{result1, result2} } -func (fake *FakeObjectStore) ListRooms(arg1 context.Context, arg2 []livekit.RoomName, arg3 []livekit.RoomID) ([]*livekit.Room, error) { +func (fake *FakeObjectStore) ListRooms(arg1 context.Context, arg2 []livekit.RoomName) ([]*livekit.Room, error) { var arg2Copy []livekit.RoomName if arg2 != nil { arg2Copy = make([]livekit.RoomName, len(arg2)) copy(arg2Copy, arg2) } - var arg3Copy []livekit.RoomID - if arg3 != nil { - arg3Copy = make([]livekit.RoomID, len(arg3)) - copy(arg3Copy, arg3) - } fake.listRoomsMutex.Lock() ret, specificReturn := fake.listRoomsReturnsOnCall[len(fake.listRoomsArgsForCall)] fake.listRoomsArgsForCall = append(fake.listRoomsArgsForCall, struct { arg1 context.Context arg2 []livekit.RoomName - arg3 []livekit.RoomID - }{arg1, arg2Copy, arg3Copy}) + }{arg1, arg2Copy}) stub := fake.ListRoomsStub fakeReturns := fake.listRoomsReturns - fake.recordInvocation("ListRooms", []interface{}{arg1, arg2Copy, arg3Copy}) + fake.recordInvocation("ListRooms", []interface{}{arg1, arg2Copy}) fake.listRoomsMutex.Unlock() if stub != nil { - return stub(arg1, arg2, arg3) + return stub(arg1, arg2) } if specificReturn { return ret.result1, ret.result2 @@ -569,17 +562,17 @@ func (fake *FakeObjectStore) ListRoomsCallCount() int { return len(fake.listRoomsArgsForCall) } -func (fake *FakeObjectStore) ListRoomsCalls(stub func(context.Context, []livekit.RoomName, []livekit.RoomID) ([]*livekit.Room, error)) { +func (fake *FakeObjectStore) ListRoomsCalls(stub func(context.Context, []livekit.RoomName) ([]*livekit.Room, error)) { fake.listRoomsMutex.Lock() defer fake.listRoomsMutex.Unlock() fake.ListRoomsStub = stub } -func (fake *FakeObjectStore) ListRoomsArgsForCall(i int) (context.Context, []livekit.RoomName, []livekit.RoomID) { +func (fake *FakeObjectStore) ListRoomsArgsForCall(i int) (context.Context, []livekit.RoomName) { fake.listRoomsMutex.RLock() defer fake.listRoomsMutex.RUnlock() argsForCall := fake.listRoomsArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 + return argsForCall.arg1, argsForCall.arg2 } func (fake *FakeObjectStore) ListRoomsReturns(result1 []*livekit.Room, result2 error) { diff --git a/pkg/service/servicefakes/fake_service_store.go b/pkg/service/servicefakes/fake_service_store.go index 3c9b280af..5c0bd4544 100644 --- a/pkg/service/servicefakes/fake_service_store.go +++ b/pkg/service/servicefakes/fake_service_store.go @@ -50,12 +50,11 @@ type FakeServiceStore struct { result1 []*livekit.ParticipantInfo result2 error } - ListRoomsStub func(context.Context, []livekit.RoomName, []livekit.RoomID) ([]*livekit.Room, error) + ListRoomsStub func(context.Context, []livekit.RoomName) ([]*livekit.Room, error) listRoomsMutex sync.RWMutex listRoomsArgsForCall []struct { arg1 context.Context arg2 []livekit.RoomName - arg3 []livekit.RoomID } listRoomsReturns struct { result1 []*livekit.Room @@ -328,30 +327,24 @@ func (fake *FakeServiceStore) ListParticipantsReturnsOnCall(i int, result1 []*li }{result1, result2} } -func (fake *FakeServiceStore) ListRooms(arg1 context.Context, arg2 []livekit.RoomName, arg3 []livekit.RoomID) ([]*livekit.Room, error) { +func (fake *FakeServiceStore) ListRooms(arg1 context.Context, arg2 []livekit.RoomName) ([]*livekit.Room, error) { var arg2Copy []livekit.RoomName if arg2 != nil { arg2Copy = make([]livekit.RoomName, len(arg2)) copy(arg2Copy, arg2) } - var arg3Copy []livekit.RoomID - if arg3 != nil { - arg3Copy = make([]livekit.RoomID, len(arg3)) - copy(arg3Copy, arg3) - } fake.listRoomsMutex.Lock() ret, specificReturn := fake.listRoomsReturnsOnCall[len(fake.listRoomsArgsForCall)] fake.listRoomsArgsForCall = append(fake.listRoomsArgsForCall, struct { arg1 context.Context arg2 []livekit.RoomName - arg3 []livekit.RoomID - }{arg1, arg2Copy, arg3Copy}) + }{arg1, arg2Copy}) stub := fake.ListRoomsStub fakeReturns := fake.listRoomsReturns - fake.recordInvocation("ListRooms", []interface{}{arg1, arg2Copy, arg3Copy}) + fake.recordInvocation("ListRooms", []interface{}{arg1, arg2Copy}) fake.listRoomsMutex.Unlock() if stub != nil { - return stub(arg1, arg2, arg3) + return stub(arg1, arg2) } if specificReturn { return ret.result1, ret.result2 @@ -365,17 +358,17 @@ func (fake *FakeServiceStore) ListRoomsCallCount() int { return len(fake.listRoomsArgsForCall) } -func (fake *FakeServiceStore) ListRoomsCalls(stub func(context.Context, []livekit.RoomName, []livekit.RoomID) ([]*livekit.Room, error)) { +func (fake *FakeServiceStore) ListRoomsCalls(stub func(context.Context, []livekit.RoomName) ([]*livekit.Room, error)) { fake.listRoomsMutex.Lock() defer fake.listRoomsMutex.Unlock() fake.ListRoomsStub = stub } -func (fake *FakeServiceStore) ListRoomsArgsForCall(i int) (context.Context, []livekit.RoomName, []livekit.RoomID) { +func (fake *FakeServiceStore) ListRoomsArgsForCall(i int) (context.Context, []livekit.RoomName) { fake.listRoomsMutex.RLock() defer fake.listRoomsMutex.RUnlock() argsForCall := fake.listRoomsArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 + return argsForCall.arg1, argsForCall.arg2 } func (fake *FakeServiceStore) ListRoomsReturns(result1 []*livekit.Room, result2 error) { diff --git a/test/scenarios.go b/test/scenarios.go index 91bde898e..d5c7878e0 100644 --- a/test/scenarios.go +++ b/test/scenarios.go @@ -202,11 +202,11 @@ func roomServiceListRoom(t *testing.T) { createCtx := contextWithToken(createRoomToken()) listCtx := contextWithToken(listRoomToken()) // create rooms - testRm, err := roomClient.CreateRoom(createCtx, &livekit.CreateRoomRequest{ + _, err := roomClient.CreateRoom(createCtx, &livekit.CreateRoomRequest{ Name: testRoom, }) require.NoError(t, err) - yourRm, err := roomClient.CreateRoom(contextWithToken(createRoomToken()), &livekit.CreateRoomRequest{ + _, err = roomClient.CreateRoom(contextWithToken(createRoomToken()), &livekit.CreateRoomRequest{ Name: "yourroom", }) require.NoError(t, err) @@ -216,7 +216,7 @@ func roomServiceListRoom(t *testing.T) { require.NoError(t, err) require.Len(t, res.Rooms, 2) }) - t.Run("list specific rooms by name", func(t *testing.T) { + t.Run("list specific rooms", func(t *testing.T) { res, err := roomClient.ListRooms(listCtx, &livekit.ListRoomsRequest{ Names: []string{"yourroom"}, }) @@ -224,29 +224,4 @@ func roomServiceListRoom(t *testing.T) { require.Len(t, res.Rooms, 1) require.Equal(t, "yourroom", res.Rooms[0].Name) }) - t.Run("list specific rooms by sid", func(t *testing.T) { - res, err := roomClient.ListRooms(listCtx, &livekit.ListRoomsRequest{ - RoomSids: []string{testRm.Sid}, - }) - require.NoError(t, err) - require.Len(t, res.Rooms, 1) - require.Equal(t, testRm.Sid, res.Rooms[0].Sid) - }) - t.Run("list specific rooms by name and sid overlap", func(t *testing.T) { - res, err := roomClient.ListRooms(listCtx, &livekit.ListRoomsRequest{ - Names: []string{testRoom}, - RoomSids: []string{testRm.Sid}, - }) - require.NoError(t, err) - require.Len(t, res.Rooms, 1) - require.Equal(t, testRm.Sid, res.Rooms[0].Sid) - }) - t.Run("list specific rooms by name and sid non-overlap", func(t *testing.T) { - res, err := roomClient.ListRooms(listCtx, &livekit.ListRoomsRequest{ - Names: []string{testRoom}, - RoomSids: []string{yourRm.Sid}, - }) - require.NoError(t, err) - require.Len(t, res.Rooms, 2) - }) }