Skip to content

perf(room-service): trim GetRoom/GetSubscription decode + add CheckMembership gate #372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vjauhari-work merged 3 commits into from
Jun 24, 2026
Merged

perf(room-service): trim GetRoom/GetSubscription decode + add CheckMembership gate #372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
952ae72
perf(room-service): project GetRoom and GetSubscription reads
claude Jun 21, 2026
71fcd97
perf(room-service): add CheckMembership existence check for gate call…
claude Jun 21, 2026
6152c44
test(room-service): assert mute/favorite persistence via direct read
claude Jun 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions room-service/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ func (h *Handler) listMembers(c *natsrouter.Context) (*model.ListRoomMembersResp
requesterAccount := c.Param("account")
roomID := c.Param("roomID")

_, err := h.store.GetSubscription(ctx, requesterAccount, roomID)
err := h.store.CheckMembership(ctx, requesterAccount, roomID)
switch {
case errors.Is(err, model.ErrSubscriptionNotFound):
return nil, errNotRoomMember
Expand Down Expand Up @@ -405,7 +405,7 @@ func (h *Handler) getRoomKey(c *natsrouter.Context) (*model.RoomKeyGetResponse,
requesterAccount := c.Param("account")
roomID := c.Param("roomID")

_, err := h.store.GetSubscription(ctx, requesterAccount, roomID)
err := h.store.CheckMembership(ctx, requesterAccount, roomID)
switch {
case errors.Is(err, model.ErrSubscriptionNotFound):
return nil, errNotRoomMember
Expand Down Expand Up @@ -475,7 +475,7 @@ func (h *Handler) requireMembershipAndGetRoom(ctx context.Context, account, room
wg.Add(2)
go func() {
defer wg.Done()
_, subErr = h.store.GetSubscription(ctx, account, roomID)
subErr = h.store.CheckMembership(ctx, account, roomID)
}()
go func() {
defer wg.Done()
Expand Down Expand Up @@ -983,7 +983,7 @@ func (h *Handler) expandChannelRefs(ctx context.Context, requester string, refs
refCtx, cancel := h.contextWithMemberListTimeout(ctx)

if ref.SiteID == h.siteID {
if _, subErr := h.store.GetSubscription(refCtx, requester, ref.RoomID); subErr != nil {
if subErr := h.store.CheckMembership(refCtx, requester, ref.RoomID); subErr != nil {
cancel()
if errors.Is(subErr, context.DeadlineExceeded) {
return nil, nil, errcode.Unavailable(fmt.Sprintf("timeout listing members of channel %s@%s", ref.RoomID, ref.SiteID))
Expand Down Expand Up @@ -1399,8 +1399,7 @@ func (h *Handler) messageReadReceipt(c *natsrouter.Context, req model.ReadReceip
)
g, gctx := errgroup.WithContext(ctx)
g.Go(func() error {
_, err := h.store.GetSubscription(gctx, requesterAccount, roomID)
subErr = err
subErr = h.store.CheckMembership(gctx, requesterAccount, roomID)
return nil
})
g.Go(func() error {
Expand Down Expand Up @@ -1467,7 +1466,7 @@ func (h *Handler) messageThreadRead(c *natsrouter.Context, req model.MessageThre
)
var g errgroup.Group
g.Go(func() error {
_, err := h.store.GetSubscription(ctx, account, roomID)
err := h.store.CheckMembership(ctx, account, roomID)
subErr = err
return err
})
Expand Down Expand Up @@ -1742,11 +1741,11 @@ func (h *Handler) roomRestricted(c *natsrouter.Context, req model.RoomRestricted
isTransition := req.Restricted && !room.Restricted

if req.Restricted && req.OwnerAccount != "" {
if _, subErr := h.store.GetSubscription(ctx, req.OwnerAccount, req.RoomID); subErr != nil {
if errors.Is(subErr, mongo.ErrNoDocuments) || errors.Is(subErr, model.ErrSubscriptionNotFound) {
if subErr := h.store.CheckMembership(ctx, req.OwnerAccount, req.RoomID); subErr != nil {
if errors.Is(subErr, model.ErrSubscriptionNotFound) {
return nil, errOwnerNotMember
}
return nil, fmt.Errorf("get owner subscription: %w", subErr)
return nil, fmt.Errorf("check owner membership: %w", subErr)
}
}
if isTransition {
Expand Down
28 changes: 14 additions & 14 deletions room-service/handler_teams_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ func TestTeamsRoomCall_Success_ExcludesSelf(t *testing.T) {
ctrl := gomock.NewController(t)
store := NewMockRoomStore(ctrl)

store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1").
Return(&model.Subscription{RoomID: "r1"}, nil)
store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1").
Return(nil)
store.EXPECT().GetRoom(gomock.Any(), "r1").
Return(&model.Room{ID: "r1", Name: "general", Type: model.RoomTypeChannel}, nil)
store.EXPECT().ListRoomMembers(gomock.Any(), "r1", nil, nil, false).
Expand Down Expand Up @@ -200,8 +200,8 @@ func TestTeamsRoomCall_RoomIDMissing(t *testing.T) {
func TestTeamsRoomCall_NotMember(t *testing.T) {
ctrl := gomock.NewController(t)
store := NewMockRoomStore(ctrl)
store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1").
Return(nil, model.ErrSubscriptionNotFound)
store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1").
Return(model.ErrSubscriptionNotFound)
store.EXPECT().GetRoom(gomock.Any(), "r1").
Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel}, nil)

Expand All @@ -213,7 +213,7 @@ func TestTeamsRoomCall_NotMember(t *testing.T) {
func TestTeamsRoomCall_NoOtherMembers(t *testing.T) {
ctrl := gomock.NewController(t)
store := NewMockRoomStore(ctrl)
store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1").Return(&model.Subscription{RoomID: "r1"}, nil)
store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1").Return(nil)
store.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel}, nil)
store.EXPECT().ListRoomMembers(gomock.Any(), "r1", nil, nil, false).
Return([]model.RoomMember{indMember("alice")}, nil)
Expand All @@ -227,7 +227,7 @@ func TestTeamsRoomCall_NoOtherMembers(t *testing.T) {
func TestTeamsRoomCall_TooManyMembers(t *testing.T) {
ctrl := gomock.NewController(t)
store := NewMockRoomStore(ctrl)
store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1").Return(&model.Subscription{RoomID: "r1"}, nil)
store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1").Return(nil)
store.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel}, nil)

members := []model.RoomMember{indMember("alice")}
Expand Down Expand Up @@ -271,7 +271,7 @@ func TestTeamsMeeting_CreatesAndPublishes(t *testing.T) {
ctrl := gomock.NewController(t)
store := NewMockRoomStore(ctrl)

store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1").Return(&model.Subscription{RoomID: "r1"}, nil)
store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1").Return(nil)
store.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Name: "general", Type: model.RoomTypeChannel}, nil)
store.EXPECT().ListRoomMembers(gomock.Any(), "r1", nil, nil, false).
Return([]model.RoomMember{indMember("alice"), indMember("bob")}, nil)
Expand Down Expand Up @@ -335,7 +335,7 @@ func TestTeamsMeeting_Idempotent_FastPathReadHit(t *testing.T) {
ctrl := gomock.NewController(t)
store := NewMockRoomStore(ctrl)

store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1").Return(&model.Subscription{RoomID: "r1"}, nil)
store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1").Return(nil)
store.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel}, nil)
// ListRoomMembers must NOT be called when a record already exists.

Expand Down Expand Up @@ -374,7 +374,7 @@ func TestTeamsMeeting_DuplicateKey_ReturnsExisting(t *testing.T) {
ctrl := gomock.NewController(t)
store := NewMockRoomStore(ctrl)

store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1").Return(&model.Subscription{RoomID: "r1"}, nil)
store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1").Return(nil)
store.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel}, nil)
store.EXPECT().ListRoomMembers(gomock.Any(), "r1", nil, nil, false).
Return([]model.RoomMember{indMember("alice")}, nil)
Expand Down Expand Up @@ -429,7 +429,7 @@ func TestTeamsMeeting_Concurrent_SingleCreateSingleMessage(t *testing.T) {
ctrl := gomock.NewController(t)
store := NewMockRoomStore(ctrl)
// Both goroutines run the membership/room/member reads; allow any count.
store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1").Return(&model.Subscription{RoomID: "r1"}, nil).AnyTimes()
store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1").Return(nil).AnyTimes()
store.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel}, nil).AnyTimes()
store.EXPECT().ListRoomMembers(gomock.Any(), "r1", nil, nil, false).
Return([]model.RoomMember{indMember("alice")}, nil).AnyTimes()
Expand Down Expand Up @@ -507,7 +507,7 @@ func TestTeamsMeeting_RoomIDMissing(t *testing.T) {
func TestTeamsMeeting_NotMember(t *testing.T) {
ctrl := gomock.NewController(t)
store := NewMockRoomStore(ctrl)
store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1").Return(nil, model.ErrSubscriptionNotFound)
store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1").Return(model.ErrSubscriptionNotFound)
store.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel}, nil)

graph := &fakeGraphClient{}
Expand All @@ -521,7 +521,7 @@ func TestTeamsMeeting_NotMember(t *testing.T) {
func TestTeamsMeeting_TooManyMembers(t *testing.T) {
ctrl := gomock.NewController(t)
store := NewMockRoomStore(ctrl)
store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1").Return(&model.Subscription{RoomID: "r1"}, nil)
store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1").Return(nil)
store.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel}, nil)
store.EXPECT().ListRoomMembers(gomock.Any(), "r1", nil, nil, false).
Return([]model.RoomMember{indMember("alice"), indMember("bob"), indMember("carol")}, nil)
Expand All @@ -538,7 +538,7 @@ func TestTeamsMeeting_TooManyMembers(t *testing.T) {
func TestTeamsMeeting_GraphCreateFails(t *testing.T) {
ctrl := gomock.NewController(t)
store := NewMockRoomStore(ctrl)
store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1").Return(&model.Subscription{RoomID: "r1"}, nil)
store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1").Return(nil)
store.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel}, nil)
store.EXPECT().ListRoomMembers(gomock.Any(), "r1", nil, nil, false).
Return([]model.RoomMember{indMember("alice")}, nil)
Expand All @@ -557,7 +557,7 @@ func TestTeamsMeeting_GraphCreateFails(t *testing.T) {
func TestTeamsMeeting_RecordReadFails(t *testing.T) {
ctrl := gomock.NewController(t)
store := NewMockRoomStore(ctrl)
store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1").Return(&model.Subscription{RoomID: "r1"}, nil)
store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1").Return(nil)
store.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel}, nil)

graph := &fakeGraphClient{}
Expand Down
Loading
Loading