diff --git a/room-service/handler.go b/room-service/handler.go index bf83b2683..7cf44b24d 100644 --- a/room-service/handler.go +++ b/room-service/handler.go @@ -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 @@ -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 @@ -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() @@ -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)) @@ -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 { @@ -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 }) @@ -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 { diff --git a/room-service/handler_teams_test.go b/room-service/handler_teams_test.go index 1b6169047..183e9fa17 100644 --- a/room-service/handler_teams_test.go +++ b/room-service/handler_teams_test.go @@ -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). @@ -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) @@ -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) @@ -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")} @@ -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) @@ -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. @@ -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) @@ -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() @@ -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{} @@ -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) @@ -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) @@ -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{} diff --git a/room-service/handler_test.go b/room-service/handler_test.go index ff11365ca..4b6718447 100644 --- a/room-service/handler_test.go +++ b/room-service/handler_test.go @@ -1075,7 +1075,7 @@ func TestHandler_AddMembers_SilentlyFiltersBotsFromChannelRefs(t *testing.T) { // Channel-ref expansion: same-site source channel returns a human + a bot. srcRef := model.ChannelRef{RoomID: "r_src", SiteID: "site-a"} - store.EXPECT().GetSubscription(gomock.Any(), "alice", "r_src").Return(&model.Subscription{}, nil) + store.EXPECT().CheckMembership(gomock.Any(), "alice", "r_src").Return(nil) store.EXPECT().ListRoomMembers(gomock.Any(), "r_src", gomock.Any(), nil, false).Return([]model.RoomMember{ {Member: model.RoomMemberEntry{Type: model.RoomMemberIndividual, Account: "bob"}}, {Member: model.RoomMemberEntry{Type: model.RoomMemberIndividual, Account: "weather.bot"}}, @@ -1347,7 +1347,7 @@ func TestHandler_AddMembers_ChannelExpansion(t *testing.T) { mc := NewMockMemberListClient(ctrl) ch := model.ChannelRef{RoomID: "ch1", SiteID: "site-a"} - store.EXPECT().GetSubscription(gomock.Any(), "alice", "ch1").Return(&model.Subscription{}, nil) + store.EXPECT().CheckMembership(gomock.Any(), "alice", "ch1").Return(nil) store.EXPECT().ListRoomMembers(gomock.Any(), "ch1", gomock.Any(), nil, false).Return([]model.RoomMember{ {Member: model.RoomMemberEntry{Type: model.RoomMemberIndividual, Account: "bob"}}, {Member: model.RoomMemberEntry{Type: model.RoomMemberIndividual, Account: "carol"}}, @@ -1367,7 +1367,7 @@ func TestHandler_AddMembers_ChannelExpansion(t *testing.T) { mc := NewMockMemberListClient(ctrl) ch := model.ChannelRef{RoomID: "ch1", SiteID: "site-a"} - store.EXPECT().GetSubscription(gomock.Any(), "alice", "ch1").Return(&model.Subscription{}, nil) + store.EXPECT().CheckMembership(gomock.Any(), "alice", "ch1").Return(nil) store.EXPECT().ListRoomMembers(gomock.Any(), "ch1", gomock.Any(), nil, false).Return([]model.RoomMember{ {Member: model.RoomMemberEntry{ID: "org1", Type: model.RoomMemberOrg}}, {Member: model.RoomMemberEntry{ID: "org2", Type: model.RoomMemberOrg}}, @@ -1387,7 +1387,7 @@ func TestHandler_AddMembers_ChannelExpansion(t *testing.T) { mc := NewMockMemberListClient(ctrl) ch := model.ChannelRef{RoomID: "ch1", SiteID: "site-a"} - store.EXPECT().GetSubscription(gomock.Any(), "alice", "ch1").Return(&model.Subscription{}, nil) + store.EXPECT().CheckMembership(gomock.Any(), "alice", "ch1").Return(nil) store.EXPECT().ListRoomMembers(gomock.Any(), "ch1", gomock.Any(), nil, false).Return([]model.RoomMember{ {Member: model.RoomMemberEntry{ID: "org1", Type: model.RoomMemberOrg}}, {Member: model.RoomMemberEntry{Type: model.RoomMemberIndividual, Account: "bob"}}, @@ -1429,7 +1429,7 @@ func TestHandler_AddMembers_ChannelExpansion(t *testing.T) { local := model.ChannelRef{RoomID: "ch-local", SiteID: "site-a"} remote := model.ChannelRef{RoomID: "ch-remote", SiteID: "site-eu"} - store.EXPECT().GetSubscription(gomock.Any(), "alice", "ch-local").Return(&model.Subscription{}, nil) + store.EXPECT().CheckMembership(gomock.Any(), "alice", "ch-local").Return(nil) store.EXPECT().ListRoomMembers(gomock.Any(), "ch-local", gomock.Any(), nil, false).Return([]model.RoomMember{ {Member: model.RoomMemberEntry{Type: model.RoomMemberIndividual, Account: "local-user"}}, }, nil) @@ -1450,7 +1450,7 @@ func TestHandler_AddMembers_ChannelExpansion(t *testing.T) { mc := NewMockMemberListClient(ctrl) ch := model.ChannelRef{RoomID: "ch1", SiteID: "site-a"} - store.EXPECT().GetSubscription(gomock.Any(), "alice", "ch1").Return(nil, model.ErrSubscriptionNotFound) + store.EXPECT().CheckMembership(gomock.Any(), "alice", "ch1").Return(model.ErrSubscriptionNotFound) h := &Handler{store: store, siteID: "site-a", maxRoomSize: 1000, memberListClient: mc} _, _, err := h.expandChannelRefs(context.Background(), "alice", []model.ChannelRef{ch}) @@ -1465,7 +1465,7 @@ func TestHandler_AddMembers_ChannelExpansion(t *testing.T) { mc := NewMockMemberListClient(ctrl) ch := model.ChannelRef{RoomID: "ch1", SiteID: "site-a"} - store.EXPECT().GetSubscription(gomock.Any(), "alice", "ch1").Return(nil, errors.New("mongo timeout")) + store.EXPECT().CheckMembership(gomock.Any(), "alice", "ch1").Return(errors.New("mongo timeout")) h := &Handler{store: store, siteID: "site-a", maxRoomSize: 1000, memberListClient: mc} _, _, err := h.expandChannelRefs(context.Background(), "alice", []model.ChannelRef{ch}) @@ -1480,8 +1480,8 @@ func TestHandler_AddMembers_ChannelExpansion(t *testing.T) { mc := NewMockMemberListClient(ctrl) ch := model.ChannelRef{RoomID: "ch-slow", SiteID: "site-a"} - store.EXPECT().GetSubscription(gomock.Any(), "alice", "ch-slow"). - Return(nil, context.DeadlineExceeded) + store.EXPECT().CheckMembership(gomock.Any(), "alice", "ch-slow"). + Return(context.DeadlineExceeded) h := &Handler{store: store, siteID: "site-a", maxRoomSize: 1000, memberListClient: mc, memberListTimeout: 10 * time.Millisecond} _, _, err := h.expandChannelRefs(context.Background(), "alice", []model.ChannelRef{ch}) @@ -1520,7 +1520,7 @@ func TestHandler_AddMembers_ChannelExpansion(t *testing.T) { mc := NewMockMemberListClient(ctrl) ch := model.ChannelRef{RoomID: "ch1", SiteID: "site-a"} - store.EXPECT().GetSubscription(gomock.Any(), "alice", "ch1").Return(&model.Subscription{}, nil) + store.EXPECT().CheckMembership(gomock.Any(), "alice", "ch1").Return(nil) store.EXPECT().ListRoomMembers(gomock.Any(), "ch1", gomock.Any(), nil, false).Return(nil, errors.New("mongo timeout")) h := &Handler{store: store, siteID: "site-a", maxRoomSize: 1000, memberListClient: mc} @@ -1620,7 +1620,7 @@ func TestHandler_AddMembers_ChannelExpansion(t *testing.T) { mc := NewMockMemberListClient(ctrl) ch := model.ChannelRef{RoomID: "ch1", SiteID: "site-a"} - store.EXPECT().GetSubscription(gomock.Any(), "alice", "ch1").Return(&model.Subscription{}, nil) + store.EXPECT().CheckMembership(gomock.Any(), "alice", "ch1").Return(nil) store.EXPECT().ListRoomMembers(gomock.Any(), "ch1", gomock.Any(), nil, false).Return([]model.RoomMember{ {Member: model.RoomMemberEntry{ID: "unknown", Type: ""}}, {Member: model.RoomMemberEntry{Type: model.RoomMemberIndividual, Account: "bob"}}, @@ -1663,8 +1663,8 @@ func TestHandler_ListMembers(t *testing.T) { name: "happy path returns members", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().ListRoomMembers(gomock.Any(), roomID, (*int)(nil), (*int)(nil), false). Return([]model.RoomMember{orgMember, existingMember}, nil) }, @@ -1678,8 +1678,8 @@ func TestHandler_ListMembers(t *testing.T) { ID: "sub-xyz", RoomID: roomID, Ts: time.Unix(3, 0).UTC(), Member: model.RoomMemberEntry{ID: "u-alice", Type: model.RoomMemberIndividual, Account: "alice"}, } - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().ListRoomMembers(gomock.Any(), roomID, (*int)(nil), (*int)(nil), false). Return([]model.RoomMember{synth}, nil) }, @@ -1692,8 +1692,8 @@ func TestHandler_ListMembers(t *testing.T) { name: "requester not a member", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(nil, fmt.Errorf("missing: %w", model.ErrSubscriptionNotFound)) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(fmt.Errorf("missing: %w", model.ErrSubscriptionNotFound)) }, want: want{errIs: errNotRoomMember}, }, @@ -1701,8 +1701,8 @@ func TestHandler_ListMembers(t *testing.T) { name: "invalid JSON body", body: []byte("{not json"), setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) }, want: want{errContains: "invalid request"}, }, @@ -1710,8 +1710,8 @@ func TestHandler_ListMembers(t *testing.T) { name: "non-positive limit: negative", body: []byte(`{"limit":-1}`), setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) }, want: want{errContains: "limit must be > 0"}, }, @@ -1719,8 +1719,8 @@ func TestHandler_ListMembers(t *testing.T) { name: "non-positive limit: zero", body: []byte(`{"limit":0}`), setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) }, want: want{errContains: "limit must be > 0"}, }, @@ -1728,8 +1728,8 @@ func TestHandler_ListMembers(t *testing.T) { name: "negative offset", body: []byte(`{"offset":-1}`), setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) }, want: want{errContains: "offset must be >= 0"}, }, @@ -1737,8 +1737,8 @@ func TestHandler_ListMembers(t *testing.T) { name: "pagination passed through", body: []byte(`{"limit":10,"offset":5}`), setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().ListRoomMembers(gomock.Any(), roomID, gomock.Any(), gomock.Any(), false). DoAndReturn(func(_ context.Context, _ string, limit, offset *int, _ bool) ([]model.RoomMember, error) { require.NotNil(t, limit) @@ -1754,8 +1754,8 @@ func TestHandler_ListMembers(t *testing.T) { name: "auth probe infra error", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(nil, fmt.Errorf("mongo exploded")) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(fmt.Errorf("mongo exploded")) }, want: want{errContains: "check room membership"}, }, @@ -1763,8 +1763,8 @@ func TestHandler_ListMembers(t *testing.T) { name: "store error on list", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().ListRoomMembers(gomock.Any(), roomID, (*int)(nil), (*int)(nil), false). Return(nil, fmt.Errorf("mongo exploded")) }, @@ -1774,8 +1774,8 @@ func TestHandler_ListMembers(t *testing.T) { name: "enrich=true passed through to store", body: []byte(`{"enrich":true}`), setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().ListRoomMembers(gomock.Any(), roomID, (*int)(nil), (*int)(nil), true). Return([]model.RoomMember{ { @@ -1837,8 +1837,8 @@ func TestHandler_ListMembers_EmptyBody(t *testing.T) { ) ctrl := gomock.NewController(t) store := NewMockRoomStore(ctrl) - store.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + store.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) store.EXPECT().ListRoomMembers(gomock.Any(), roomID, (*int)(nil), (*int)(nil), false). Return([]model.RoomMember{{ID: "rm1", RoomID: roomID, Member: model.RoomMemberEntry{ID: "alice", Type: model.RoomMemberIndividual, Account: "alice"}}}, nil) @@ -3053,8 +3053,8 @@ func TestHandler_handleMessageReadReceipt(t *testing.T) { name: "happy path", req: req, prep: func(s setup) { - s.store.EXPECT().GetSubscription(gomock.Any(), account, roomID). - Return(&model.Subscription{}, nil) + s.store.EXPECT().CheckMembership(gomock.Any(), account, roomID). + Return(nil) s.reader.EXPECT().GetMessageRoomAndCreatedAt(gomock.Any(), messageID). Return(roomID, createdAt, account, true, nil) s.store.EXPECT().ListReadReceipts(gomock.Any(), roomID, createdAt, account, gomock.Any()). @@ -3072,8 +3072,8 @@ func TestHandler_handleMessageReadReceipt(t *testing.T) { name: "empty readers", req: req, prep: func(s setup) { - s.store.EXPECT().GetSubscription(gomock.Any(), account, roomID). - Return(&model.Subscription{}, nil) + s.store.EXPECT().CheckMembership(gomock.Any(), account, roomID). + Return(nil) s.reader.EXPECT().GetMessageRoomAndCreatedAt(gomock.Any(), messageID). Return(roomID, createdAt, account, true, nil) s.store.EXPECT().ListReadReceipts(gomock.Any(), roomID, createdAt, account, gomock.Any()). @@ -3090,8 +3090,8 @@ func TestHandler_handleMessageReadReceipt(t *testing.T) { name: "not a room member", req: req, prep: func(s setup) { - s.store.EXPECT().GetSubscription(gomock.Any(), account, roomID). - Return(nil, model.ErrSubscriptionNotFound) + s.store.EXPECT().CheckMembership(gomock.Any(), account, roomID). + Return(model.ErrSubscriptionNotFound) s.reader.EXPECT().GetMessageRoomAndCreatedAt(gomock.Any(), messageID). Return(roomID, createdAt, account, true, nil).AnyTimes() }, @@ -3101,8 +3101,8 @@ func TestHandler_handleMessageReadReceipt(t *testing.T) { name: "message not found", req: req, prep: func(s setup) { - s.store.EXPECT().GetSubscription(gomock.Any(), account, roomID). - Return(&model.Subscription{}, nil) + s.store.EXPECT().CheckMembership(gomock.Any(), account, roomID). + Return(nil) s.reader.EXPECT().GetMessageRoomAndCreatedAt(gomock.Any(), messageID). Return("", time.Time{}, "", false, nil) }, @@ -3112,8 +3112,8 @@ func TestHandler_handleMessageReadReceipt(t *testing.T) { name: "message in another room", req: req, prep: func(s setup) { - s.store.EXPECT().GetSubscription(gomock.Any(), account, roomID). - Return(&model.Subscription{}, nil) + s.store.EXPECT().CheckMembership(gomock.Any(), account, roomID). + Return(nil) s.reader.EXPECT().GetMessageRoomAndCreatedAt(gomock.Any(), messageID). Return("other-room", createdAt, account, true, nil) }, @@ -3123,8 +3123,8 @@ func TestHandler_handleMessageReadReceipt(t *testing.T) { name: "not the sender", req: req, prep: func(s setup) { - s.store.EXPECT().GetSubscription(gomock.Any(), account, roomID). - Return(&model.Subscription{}, nil) + s.store.EXPECT().CheckMembership(gomock.Any(), account, roomID). + Return(nil) s.reader.EXPECT().GetMessageRoomAndCreatedAt(gomock.Any(), messageID). Return(roomID, createdAt, "bob", true, nil) }, @@ -3134,8 +3134,8 @@ func TestHandler_handleMessageReadReceipt(t *testing.T) { name: "store error on subscription", req: req, prep: func(s setup) { - s.store.EXPECT().GetSubscription(gomock.Any(), account, roomID). - Return(nil, fmt.Errorf("db down")) + s.store.EXPECT().CheckMembership(gomock.Any(), account, roomID). + Return(fmt.Errorf("db down")) s.reader.EXPECT().GetMessageRoomAndCreatedAt(gomock.Any(), messageID). Return(roomID, createdAt, account, true, nil).AnyTimes() }, @@ -3145,8 +3145,8 @@ func TestHandler_handleMessageReadReceipt(t *testing.T) { name: "store error on message lookup", req: req, prep: func(s setup) { - s.store.EXPECT().GetSubscription(gomock.Any(), account, roomID). - Return(&model.Subscription{}, nil) + s.store.EXPECT().CheckMembership(gomock.Any(), account, roomID). + Return(nil) s.reader.EXPECT().GetMessageRoomAndCreatedAt(gomock.Any(), messageID). Return("", time.Time{}, "", false, fmt.Errorf("cass down")) }, @@ -3156,8 +3156,8 @@ func TestHandler_handleMessageReadReceipt(t *testing.T) { name: "store error on aggregation", req: req, prep: func(s setup) { - s.store.EXPECT().GetSubscription(gomock.Any(), account, roomID). - Return(&model.Subscription{}, nil) + s.store.EXPECT().CheckMembership(gomock.Any(), account, roomID). + Return(nil) s.reader.EXPECT().GetMessageRoomAndCreatedAt(gomock.Any(), messageID). Return(roomID, createdAt, account, true, nil) s.store.EXPECT().ListReadReceipts(gomock.Any(), roomID, createdAt, account, gomock.Any()). @@ -3335,17 +3335,6 @@ func baseThreadSub(account, roomID, parent, threadRoomID string) *model.ThreadSu } } -func baseSubForThreadRead(account, roomID string, threadUnread []string, alert bool) *model.Subscription { - return &model.Subscription{ - User: model.SubscriptionUser{ID: "u-" + account, Account: account}, - RoomID: roomID, - SiteID: "site-a", - JoinedAt: time.Now().UTC().Add(-time.Hour), - ThreadUnread: threadUnread, - Alert: alert, - } -} - func TestHandler_MessageThreadRead_EmptyThreadID(t *testing.T) { f := newThreadReadFixture(t) _, err := f.handler.messageThreadRead(ctxParams(map[string]string{"account": "alice", "roomID": "r1"}), model.MessageThreadReadRequest{ThreadID: ""}) @@ -3354,8 +3343,8 @@ func TestHandler_MessageThreadRead_EmptyThreadID(t *testing.T) { func TestHandler_MessageThreadRead_NotRoomMember(t *testing.T) { f := newThreadReadFixture(t) - f.store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). - Return(nil, model.ErrSubscriptionNotFound) + f.store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1"). + Return(model.ErrSubscriptionNotFound) f.store.EXPECT().GetThreadSubscriptionByParent(gomock.Any(), "alice", "p1", "r1"). Return(baseThreadSub("alice", "r1", "p1", "tr1"), nil).AnyTimes() f.store.EXPECT().GetUserSiteID(gomock.Any(), "alice").Return("site-a", nil).AnyTimes() @@ -3367,8 +3356,8 @@ func TestHandler_MessageThreadRead_NotRoomMember(t *testing.T) { func TestHandler_MessageThreadRead_ThreadSubNotFound(t *testing.T) { f := newThreadReadFixture(t) - f.store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). - Return(baseSubForThreadRead("alice", "r1", []string{"p1"}, true), nil) + f.store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1"). + Return(nil) f.store.EXPECT().GetThreadSubscriptionByParent(gomock.Any(), "alice", "p1", "r1"). Return(nil, model.ErrThreadSubscriptionNotFound) f.store.EXPECT().GetUserSiteID(gomock.Any(), "alice").Return("site-a", nil).AnyTimes() @@ -3386,8 +3375,8 @@ func TestHandler_MessageThreadRead_ThreadSubNotFound(t *testing.T) { // Simulate by returning context.Canceled on the siblings. func TestHandler_MessageThreadRead_ThreadSubNotFound_SiblingsCancelled(t *testing.T) { f := newThreadReadFixture(t) - f.store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). - Return(nil, context.Canceled).AnyTimes() + f.store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1"). + Return(context.Canceled).AnyTimes() f.store.EXPECT().GetThreadSubscriptionByParent(gomock.Any(), "alice", "p1", "r1"). Return(nil, model.ErrThreadSubscriptionNotFound) f.store.EXPECT().GetUserSiteID(gomock.Any(), "alice"). @@ -3400,8 +3389,8 @@ func TestHandler_MessageThreadRead_ThreadSubNotFound_SiblingsCancelled(t *testin func TestHandler_MessageThreadRead_BothMiss_RoomNotMemberWins(t *testing.T) { for i := 0; i < 20; i++ { f := newThreadReadFixture(t) - f.store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). - Return(nil, model.ErrSubscriptionNotFound) + f.store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1"). + Return(model.ErrSubscriptionNotFound) f.store.EXPECT().GetThreadSubscriptionByParent(gomock.Any(), "alice", "p1", "r1"). Return(nil, model.ErrThreadSubscriptionNotFound).AnyTimes() f.store.EXPECT().GetUserSiteID(gomock.Any(), "alice").Return("site-a", nil).AnyTimes() @@ -3414,8 +3403,8 @@ func TestHandler_MessageThreadRead_BothMiss_RoomNotMemberWins(t *testing.T) { func TestHandler_MessageThreadRead_HappyAlertClears(t *testing.T) { f := newThreadReadFixture(t) withNopFloor(f) - f.store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). - Return(baseSubForThreadRead("alice", "r1", []string{"p1"}, true), nil) + f.store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1"). + Return(nil) f.store.EXPECT().GetThreadSubscriptionByParent(gomock.Any(), "alice", "p1", "r1"). Return(baseThreadSub("alice", "r1", "p1", "tr1"), nil) f.store.EXPECT().UpdateSubscriptionThreadRead(gomock.Any(), "r1", "alice", "p1"). @@ -3433,8 +3422,8 @@ func TestHandler_MessageThreadRead_HappyAlertClears(t *testing.T) { func TestHandler_MessageThreadRead_HappyAlertStays(t *testing.T) { f := newThreadReadFixture(t) withNopFloor(f) - f.store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). - Return(baseSubForThreadRead("alice", "r1", []string{"p1", "p2"}, true), nil) + f.store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1"). + Return(nil) f.store.EXPECT().GetThreadSubscriptionByParent(gomock.Any(), "alice", "p1", "r1"). Return(baseThreadSub("alice", "r1", "p1", "tr1"), nil) f.store.EXPECT().UpdateSubscriptionThreadRead(gomock.Any(), "r1", "alice", "p1"). @@ -3450,8 +3439,8 @@ func TestHandler_MessageThreadRead_HappyAlertStays(t *testing.T) { func TestHandler_MessageThreadRead_IdempotentIDNotInArray(t *testing.T) { f := newThreadReadFixture(t) withNopFloor(f) - f.store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). - Return(baseSubForThreadRead("alice", "r1", []string{"p2"}, true), nil) + f.store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1"). + Return(nil) f.store.EXPECT().GetThreadSubscriptionByParent(gomock.Any(), "alice", "p1", "r1"). Return(baseThreadSub("alice", "r1", "p1", "tr1"), nil) f.store.EXPECT().UpdateSubscriptionThreadRead(gomock.Any(), "r1", "alice", "p1"). @@ -3467,8 +3456,8 @@ func TestHandler_MessageThreadRead_IdempotentIDNotInArray(t *testing.T) { func TestHandler_MessageThreadRead_AlertAlreadyFalse(t *testing.T) { f := newThreadReadFixture(t) withNopFloor(f) - f.store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). - Return(baseSubForThreadRead("alice", "r1", []string{"p1"}, false), nil) + f.store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1"). + Return(nil) f.store.EXPECT().GetThreadSubscriptionByParent(gomock.Any(), "alice", "p1", "r1"). Return(baseThreadSub("alice", "r1", "p1", "tr1"), nil) f.store.EXPECT().UpdateSubscriptionThreadRead(gomock.Any(), "r1", "alice", "p1"). @@ -3484,8 +3473,8 @@ func TestHandler_MessageThreadRead_AlertAlreadyFalse(t *testing.T) { func TestHandler_MessageThreadRead_CrossSite_PublishesOutbox(t *testing.T) { f := newThreadReadFixture(t) withNopFloor(f) - f.store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). - Return(baseSubForThreadRead("alice", "r1", []string{"p1", "p2"}, true), nil) + f.store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1"). + Return(nil) f.store.EXPECT().GetThreadSubscriptionByParent(gomock.Any(), "alice", "p1", "r1"). Return(baseThreadSub("alice", "r1", "p1", "tr1"), nil) f.store.EXPECT().UpdateSubscriptionThreadRead(gomock.Any(), "r1", "alice", "p1"). @@ -3521,8 +3510,8 @@ func TestHandler_MessageThreadRead_CrossSite_PublishesOutbox(t *testing.T) { func TestHandler_MessageThreadRead_GetUserSiteID_Empty(t *testing.T) { f := newThreadReadFixture(t) withNopFloor(f) - f.store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). - Return(baseSubForThreadRead("alice", "r1", []string{"p1"}, true), nil) + f.store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1"). + Return(nil) f.store.EXPECT().GetThreadSubscriptionByParent(gomock.Any(), "alice", "p1", "r1"). Return(baseThreadSub("alice", "r1", "p1", "tr1"), nil) f.store.EXPECT().UpdateSubscriptionThreadRead(gomock.Any(), "r1", "alice", gomock.Any()). @@ -3538,8 +3527,8 @@ func TestHandler_MessageThreadRead_GetUserSiteID_Empty(t *testing.T) { func TestHandler_MessageThreadRead_GetUserSiteID_Error(t *testing.T) { f := newThreadReadFixture(t) - f.store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). - Return(baseSubForThreadRead("alice", "r1", []string{"p1"}, true), nil) + f.store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1"). + Return(nil) f.store.EXPECT().GetThreadSubscriptionByParent(gomock.Any(), "alice", "p1", "r1"). Return(baseThreadSub("alice", "r1", "p1", "tr1"), nil) f.store.EXPECT().GetUserSiteID(gomock.Any(), "alice").Return("", fmt.Errorf("boom")) @@ -3557,8 +3546,8 @@ func TestHandler_MessageThreadRead_GetUserSiteID_Error(t *testing.T) { func TestHandler_MessageThreadRead_OutboxPublishError(t *testing.T) { f := newThreadReadFixture(t) f.publishCallErr = fmt.Errorf("nats down") - f.store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). - Return(baseSubForThreadRead("alice", "r1", []string{"p1"}, true), nil) + f.store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1"). + Return(nil) f.store.EXPECT().GetThreadSubscriptionByParent(gomock.Any(), "alice", "p1", "r1"). Return(baseThreadSub("alice", "r1", "p1", "tr1"), nil) f.store.EXPECT().UpdateSubscriptionThreadRead(gomock.Any(), "r1", "alice", gomock.Any()). @@ -3574,8 +3563,8 @@ func TestHandler_MessageThreadRead_OutboxPublishError(t *testing.T) { func TestHandler_MessageThreadRead_UpdateSubscriptionError(t *testing.T) { f := newThreadReadFixture(t) - f.store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). - Return(baseSubForThreadRead("alice", "r1", []string{"p1"}, true), nil) + f.store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1"). + Return(nil) f.store.EXPECT().GetThreadSubscriptionByParent(gomock.Any(), "alice", "p1", "r1"). Return(baseThreadSub("alice", "r1", "p1", "tr1"), nil) f.store.EXPECT().GetUserSiteID(gomock.Any(), "alice").Return("site-a", nil) @@ -3591,8 +3580,8 @@ func TestHandler_MessageThreadRead_UpdateSubscriptionError(t *testing.T) { func TestHandler_MessageThreadRead_UpdateThreadSubscriptionError(t *testing.T) { f := newThreadReadFixture(t) - f.store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). - Return(baseSubForThreadRead("alice", "r1", []string{"p1"}, true), nil) + f.store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1"). + Return(nil) f.store.EXPECT().GetThreadSubscriptionByParent(gomock.Any(), "alice", "p1", "r1"). Return(baseThreadSub("alice", "r1", "p1", "tr1"), nil) f.store.EXPECT().GetUserSiteID(gomock.Any(), "alice").Return("site-a", nil) @@ -3619,8 +3608,8 @@ func baseThreadRoomForFloor(threadRoomID string) *model.ThreadRoom { } func fullThreadReadSetup(f *threadReadFixture, tsub *model.ThreadSubscription) { - f.store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). - Return(baseSubForThreadRead("alice", "r1", []string{"p1"}, true), nil) + f.store.EXPECT().CheckMembership(gomock.Any(), "alice", "r1"). + Return(nil) f.store.EXPECT().GetThreadSubscriptionByParent(gomock.Any(), "alice", "p1", "r1"). Return(tsub, nil) f.store.EXPECT().UpdateSubscriptionThreadRead(gomock.Any(), "r1", "alice", "p1"). @@ -3951,8 +3940,8 @@ func TestHandler_natsGetRoomKey(t *testing.T) { name: "current version, happy path", body: []byte(`{}`), setup: func(t *testing.T, store *MockRoomStore, ks *MockRoomKeyStore) { - store.EXPECT().GetSubscription(gomock.Any(), account, roomID). - Return(&model.Subscription{}, nil) + store.EXPECT().CheckMembership(gomock.Any(), account, roomID). + Return(nil) ks.EXPECT().Get(gomock.Any(), roomID).Return(sampleVersioned, nil) }, want: want{replyJSON: `{"roomId":"room-1","version":7,"privateKey":"QkJCQkJCQkJCQkJCQkJCQkJCQkJCQkJCQkJCQkJCQkI="}`}, @@ -3961,8 +3950,8 @@ func TestHandler_natsGetRoomKey(t *testing.T) { name: "explicit version, happy path", body: []byte(`{"version":3}`), setup: func(t *testing.T, store *MockRoomStore, ks *MockRoomKeyStore) { - store.EXPECT().GetSubscription(gomock.Any(), account, roomID). - Return(&model.Subscription{}, nil) + store.EXPECT().CheckMembership(gomock.Any(), account, roomID). + Return(nil) ks.EXPECT().GetByVersion(gomock.Any(), roomID, 3).Return(&sampleKey, nil) }, want: want{replyJSON: `{"roomId":"room-1","version":3,"privateKey":"QkJCQkJCQkJCQkJCQkJCQkJCQkJCQkJCQkJCQkJCQkI="}`}, @@ -3971,8 +3960,8 @@ func TestHandler_natsGetRoomKey(t *testing.T) { name: "not a member", body: []byte(`{}`), setup: func(t *testing.T, store *MockRoomStore, ks *MockRoomKeyStore) { - store.EXPECT().GetSubscription(gomock.Any(), account, roomID). - Return(nil, model.ErrSubscriptionNotFound) + store.EXPECT().CheckMembership(gomock.Any(), account, roomID). + Return(model.ErrSubscriptionNotFound) }, want: want{errSubstr: "only room members"}, }, @@ -3980,8 +3969,8 @@ func TestHandler_natsGetRoomKey(t *testing.T) { name: "current key absent", body: []byte(`{}`), setup: func(t *testing.T, store *MockRoomStore, ks *MockRoomKeyStore) { - store.EXPECT().GetSubscription(gomock.Any(), account, roomID). - Return(&model.Subscription{}, nil) + store.EXPECT().CheckMembership(gomock.Any(), account, roomID). + Return(nil) ks.EXPECT().Get(gomock.Any(), roomID).Return(nil, nil) }, want: want{errSubstr: "room key not available"}, @@ -3990,8 +3979,8 @@ func TestHandler_natsGetRoomKey(t *testing.T) { name: "historical version absent", body: []byte(`{"version":1}`), setup: func(t *testing.T, store *MockRoomStore, ks *MockRoomKeyStore) { - store.EXPECT().GetSubscription(gomock.Any(), account, roomID). - Return(&model.Subscription{}, nil) + store.EXPECT().CheckMembership(gomock.Any(), account, roomID). + Return(nil) ks.EXPECT().GetByVersion(gomock.Any(), roomID, 1).Return(nil, nil) }, want: want{errSubstr: "room key not available"}, @@ -4000,8 +3989,8 @@ func TestHandler_natsGetRoomKey(t *testing.T) { name: "store error", body: []byte(`{}`), setup: func(t *testing.T, store *MockRoomStore, ks *MockRoomKeyStore) { - store.EXPECT().GetSubscription(gomock.Any(), account, roomID). - Return(&model.Subscription{}, nil) + store.EXPECT().CheckMembership(gomock.Any(), account, roomID). + Return(nil) ks.EXPECT().Get(gomock.Any(), roomID).Return(nil, errors.New("valkey down")) }, want: want{errSubstr: "get room key:"}, @@ -4010,8 +3999,8 @@ func TestHandler_natsGetRoomKey(t *testing.T) { name: "store error on explicit version", body: []byte(`{"version":5}`), setup: func(t *testing.T, store *MockRoomStore, ks *MockRoomKeyStore) { - store.EXPECT().GetSubscription(gomock.Any(), account, roomID). - Return(&model.Subscription{}, nil) + store.EXPECT().CheckMembership(gomock.Any(), account, roomID). + Return(nil) ks.EXPECT().GetByVersion(gomock.Any(), roomID, 5).Return(nil, errors.New("valkey down")) }, want: want{errSubstr: "get room key:"}, @@ -4020,8 +4009,8 @@ func TestHandler_natsGetRoomKey(t *testing.T) { name: "malformed body", body: []byte(`not-json`), setup: func(t *testing.T, store *MockRoomStore, ks *MockRoomKeyStore) { - store.EXPECT().GetSubscription(gomock.Any(), account, roomID). - Return(&model.Subscription{}, nil) + store.EXPECT().CheckMembership(gomock.Any(), account, roomID). + Return(nil) }, want: want{errSubstr: "invalid request"}, }, @@ -4067,7 +4056,7 @@ func TestHandler_GetRoomKey_EmptyBody(t *testing.T) { ctrl := gomock.NewController(t) store := NewMockRoomStore(ctrl) ks := NewMockRoomKeyStore(ctrl) - store.EXPECT().GetSubscription(gomock.Any(), account, roomID).Return(&model.Subscription{}, nil) + store.EXPECT().CheckMembership(gomock.Any(), account, roomID).Return(nil) ks.EXPECT().Get(gomock.Any(), roomID).Return(sampleVersioned, nil) h := NewHandler(store, ks, nil, nil, siteID, 1000, 500, 5*time.Second, 5, nil, nil, nil, 0) @@ -4275,7 +4264,7 @@ func TestHandleRoomRestricted_Validation(t *testing.T) { setupStore: func(s *MockRoomStore) { s.EXPECT().GetUser(gomock.Any(), "admin1").Return(&model.User{Account: "admin1", Roles: []model.UserRole{model.UserRoleAdmin}}, nil) s.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel, Restricted: true, UserCount: 10}, nil) - s.EXPECT().GetSubscription(gomock.Any(), "nonmember", "r1").Return(nil, mongo.ErrNoDocuments) + s.EXPECT().CheckMembership(gomock.Any(), "nonmember", "r1").Return(model.ErrSubscriptionNotFound) }, wantErr: errOwnerNotMember, }, @@ -4299,7 +4288,7 @@ func TestHandleRoomRestricted_Validation(t *testing.T) { setupStore: func(s *MockRoomStore) { s.EXPECT().GetUser(gomock.Any(), "admin1").Return(&model.User{Account: "admin1", Roles: []model.UserRole{model.UserRoleAdmin}}, nil) s.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel, Restricted: false, UserCount: 3}, nil) - s.EXPECT().GetSubscription(gomock.Any(), "owner1", "r1").Return(&model.Subscription{}, nil) + s.EXPECT().CheckMembership(gomock.Any(), "owner1", "r1").Return(nil) }, wantErr: errNotEnoughMembers, }, @@ -4312,7 +4301,7 @@ func TestHandleRoomRestricted_Validation(t *testing.T) { setupStore: func(s *MockRoomStore) { s.EXPECT().GetUser(gomock.Any(), "admin1").Return(&model.User{Account: "admin1", Roles: []model.UserRole{model.UserRoleAdmin}}, nil) s.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel, Restricted: false, UserCount: 10}, nil) - s.EXPECT().GetSubscription(gomock.Any(), "owner1", "r1").Return(&model.Subscription{}, nil) + s.EXPECT().CheckMembership(gomock.Any(), "owner1", "r1").Return(nil) happyPathRestrictedSuccessSetup(s) }, wantErr: nil, @@ -4338,7 +4327,7 @@ func TestHandleRoomRestricted_Validation(t *testing.T) { setupStore: func(s *MockRoomStore) { s.EXPECT().GetUser(gomock.Any(), "admin1").Return(&model.User{Account: "admin1", Roles: []model.UserRole{model.UserRoleAdmin}}, nil) s.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel, Restricted: true, UserCount: 2}, nil) - s.EXPECT().GetSubscription(gomock.Any(), "owner2", "r1").Return(&model.Subscription{}, nil) + s.EXPECT().CheckMembership(gomock.Any(), "owner2", "r1").Return(nil) happyPathRestrictedSuccessSetup(s) }, wantErr: nil, @@ -4394,8 +4383,8 @@ func TestHandler_ListMemberStatuses(t *testing.T) { name: "default limit 3, happy path", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 10}, nil) s.EXPECT().ListMemberStatuses(gomock.Any(), roomID, 3).Return(stub, nil) @@ -4410,8 +4399,8 @@ func TestHandler_ListMemberStatuses(t *testing.T) { name: "nil limit clamped to small UserCount", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 2}, nil) s.EXPECT().ListMemberStatuses(gomock.Any(), roomID, 2).Return(stub, nil) @@ -4425,8 +4414,8 @@ func TestHandler_ListMemberStatuses(t *testing.T) { name: "nil limit + empty room returns empty without store call", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 0}, nil) }, @@ -4436,8 +4425,8 @@ func TestHandler_ListMemberStatuses(t *testing.T) { name: "explicit limit passes through", body: []byte(`{"limit":7}`), setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 10}, nil) s.EXPECT().ListMemberStatuses(gomock.Any(), roomID, 7).Return(stub, nil) @@ -4452,8 +4441,8 @@ func TestHandler_ListMemberStatuses(t *testing.T) { name: "requester not a member", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(nil, fmt.Errorf("missing: %w", model.ErrSubscriptionNotFound)) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(fmt.Errorf("missing: %w", model.ErrSubscriptionNotFound)) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 5, AppCount: 2}, nil).AnyTimes() }, @@ -4468,8 +4457,8 @@ func TestHandler_ListMemberStatuses(t *testing.T) { name: "not-member takes precedence over GetRoom error", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(nil, fmt.Errorf("missing: %w", model.ErrSubscriptionNotFound)) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(fmt.Errorf("missing: %w", model.ErrSubscriptionNotFound)) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(nil, fmt.Errorf("mongo exploded")) }, @@ -4479,8 +4468,8 @@ func TestHandler_ListMemberStatuses(t *testing.T) { name: "GetSubscription infra error", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(nil, fmt.Errorf("mongo exploded")) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(fmt.Errorf("mongo exploded")) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 5, AppCount: 2}, nil).AnyTimes() }, @@ -4490,8 +4479,8 @@ func TestHandler_ListMemberStatuses(t *testing.T) { name: "limit zero", body: []byte(`{"limit":0}`), setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID).Return(&model.Room{ID: roomID, UserCount: 10}, nil) }, want: want{errIs: errMemberStatusesLimitInvalid}, @@ -4500,8 +4489,8 @@ func TestHandler_ListMemberStatuses(t *testing.T) { name: "limit negative", body: []byte(`{"limit":-1}`), setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID).Return(&model.Room{ID: roomID, UserCount: 10}, nil) }, want: want{errIs: errMemberStatusesLimitInvalid}, @@ -4510,8 +4499,8 @@ func TestHandler_ListMemberStatuses(t *testing.T) { name: "limit exceeds room.UserCount", body: []byte(`{"limit":11}`), setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID).Return(&model.Room{ID: roomID, UserCount: 10}, nil) }, want: want{errIs: errMemberStatusesLimitInvalid}, @@ -4520,8 +4509,8 @@ func TestHandler_ListMemberStatuses(t *testing.T) { name: "limit equal to room.UserCount is valid", body: []byte(`{"limit":10}`), setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID).Return(&model.Room{ID: roomID, UserCount: 10}, nil) s.EXPECT().ListMemberStatuses(gomock.Any(), roomID, 10).Return(stub, nil) }, @@ -4531,8 +4520,8 @@ func TestHandler_ListMemberStatuses(t *testing.T) { name: "GetRoom errors", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID).Return(nil, fmt.Errorf("mongo exploded")) }, want: want{errContains: "get room"}, @@ -4541,8 +4530,8 @@ func TestHandler_ListMemberStatuses(t *testing.T) { name: "store errors", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID).Return(&model.Room{ID: roomID, UserCount: 10}, nil) s.EXPECT().ListMemberStatuses(gomock.Any(), roomID, 3). Return(nil, fmt.Errorf("mongo exploded")) @@ -5491,8 +5480,8 @@ func TestHandler_ListMentionableSubscriptions(t *testing.T) { name: "default limit 3, empty filter, happy path", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 5, AppCount: 2}, nil) s.EXPECT(). @@ -5508,8 +5497,8 @@ func TestHandler_ListMentionableSubscriptions(t *testing.T) { name: "nil limit clamped to small UserCount+AppCount", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 1, AppCount: 1}, nil) s.EXPECT(). @@ -5523,8 +5512,8 @@ func TestHandler_ListMentionableSubscriptions(t *testing.T) { name: "nil limit + empty room returns empty without store call", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 0, AppCount: 0}, nil) }, @@ -5534,8 +5523,8 @@ func TestHandler_ListMentionableSubscriptions(t *testing.T) { name: "explicit limit and filter passed through", body: []byte(`{"limit":3,"filter":"bo"}`), setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 5, AppCount: 2}, nil) s.EXPECT(). @@ -5548,8 +5537,8 @@ func TestHandler_ListMentionableSubscriptions(t *testing.T) { name: "regex metacharacters in filter are escaped", body: []byte(`{"limit":3,"filter":"a.b(c"}`), setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 5, AppCount: 2}, nil) s.EXPECT(). @@ -5566,8 +5555,8 @@ func TestHandler_ListMentionableSubscriptions(t *testing.T) { name: "requester not a member", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(nil, fmt.Errorf("missing: %w", model.ErrSubscriptionNotFound)) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(fmt.Errorf("missing: %w", model.ErrSubscriptionNotFound)) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 5, AppCount: 2}, nil).AnyTimes() }, @@ -5582,8 +5571,8 @@ func TestHandler_ListMentionableSubscriptions(t *testing.T) { name: "not-member takes precedence over GetRoom error", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(nil, fmt.Errorf("missing: %w", model.ErrSubscriptionNotFound)) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(fmt.Errorf("missing: %w", model.ErrSubscriptionNotFound)) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(nil, fmt.Errorf("mongo exploded")) }, @@ -5593,8 +5582,8 @@ func TestHandler_ListMentionableSubscriptions(t *testing.T) { name: "GetSubscription infra error", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(nil, fmt.Errorf("mongo exploded")) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(fmt.Errorf("mongo exploded")) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 5, AppCount: 2}, nil).AnyTimes() }, @@ -5604,8 +5593,8 @@ func TestHandler_ListMentionableSubscriptions(t *testing.T) { name: "limit zero", body: []byte(`{"limit":0}`), setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 5, AppCount: 2}, nil) }, @@ -5615,8 +5604,8 @@ func TestHandler_ListMentionableSubscriptions(t *testing.T) { name: "limit negative", body: []byte(`{"limit":-1}`), setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 5, AppCount: 2}, nil) }, @@ -5626,8 +5615,8 @@ func TestHandler_ListMentionableSubscriptions(t *testing.T) { name: "limit exceeds UserCount + AppCount", body: []byte(`{"limit":8}`), setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 5, AppCount: 2}, nil) }, @@ -5637,8 +5626,8 @@ func TestHandler_ListMentionableSubscriptions(t *testing.T) { name: "limit at cap is accepted", body: []byte(`{"limit":7}`), setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 5, AppCount: 2}, nil) s.EXPECT(). @@ -5651,8 +5640,8 @@ func TestHandler_ListMentionableSubscriptions(t *testing.T) { name: "GetRoom errors", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID).Return(nil, fmt.Errorf("mongo exploded")) }, want: want{errContains: "get room"}, @@ -5661,8 +5650,8 @@ func TestHandler_ListMentionableSubscriptions(t *testing.T) { name: "store errors", body: nil, setupMock: func(s *MockRoomStore) { - s.EXPECT().GetSubscription(gomock.Any(), requester, roomID). - Return(&model.Subscription{User: model.SubscriptionUser{Account: requester}, RoomID: roomID}, nil) + s.EXPECT().CheckMembership(gomock.Any(), requester, roomID). + Return(nil) s.EXPECT().GetRoom(gomock.Any(), roomID). Return(&model.Room{ID: roomID, UserCount: 5, AppCount: 2}, nil) s.EXPECT(). diff --git a/room-service/integration_test.go b/room-service/integration_test.go index 6568c491d..daa96cf18 100644 --- a/room-service/integration_test.go +++ b/room-service/integration_test.go @@ -128,6 +128,94 @@ func TestMongoStore_Integration(t *testing.T) { assert.Error(t, err, "expected error for missing subscription") } +// TestMongoStore_CheckMembership_Integration verifies the projected existence +// check: nil when subscribed, model.ErrSubscriptionNotFound when not. +func TestMongoStore_CheckMembership_Integration(t *testing.T) { + ctx := context.Background() + db := setupMongo(t) + store := NewMongoStore(db) + + mustInsertSub(t, db, &model.Subscription{ + ID: "s1", + User: model.SubscriptionUser{ID: "u1", Account: "alice"}, + RoomID: "r1", + Roles: []model.Role{model.RoleMember}, + }) + + assert.NoError(t, store.CheckMembership(ctx, "alice", "r1")) + + err := store.CheckMembership(ctx, "bob", "r1") + assert.ErrorIs(t, err, model.ErrSubscriptionNotFound) + + err = store.CheckMembership(ctx, "alice", "r2") + assert.ErrorIs(t, err, model.ErrSubscriptionNotFound) +} + +// TestMongoStore_GetRoom_ProjectionFields_Integration pins the field set that +// GetRoom's projection must return: every Room field read by any handler call +// site. Dropping one from the projection would silently zero it here, so this +// test is the guard for the projected read path. +func TestMongoStore_GetRoom_ProjectionFields_Integration(t *testing.T) { + ctx := context.Background() + db := setupMongo(t) + store := NewMongoStore(db) + + lastMsg := time.Now().UTC().Add(-time.Hour).Truncate(time.Millisecond) + minSeen := time.Now().UTC().Add(-2 * time.Hour).Truncate(time.Millisecond) + mustInsertRoom(t, db, &model.Room{ + ID: "rproj", Name: "proj-room", Type: model.RoomTypeChannel, SiteID: "site-a", + UserCount: 7, AppCount: 3, Restricted: true, ExternalAccess: true, + LastMsgAt: &lastMsg, MinUserLastSeenAt: &minSeen, LastMsgID: "m123", + }) + + got, err := store.GetRoom(ctx, "rproj") + require.NoError(t, err) + assert.Equal(t, "rproj", got.ID) + assert.Equal(t, "proj-room", got.Name) + assert.Equal(t, model.RoomTypeChannel, got.Type) + assert.Equal(t, 7, got.UserCount) + assert.Equal(t, 3, got.AppCount) + assert.True(t, got.Restricted) + assert.True(t, got.ExternalAccess) + require.NotNil(t, got.LastMsgAt) + assert.WithinDuration(t, lastMsg, *got.LastMsgAt, time.Second) + require.NotNil(t, got.MinUserLastSeenAt) + assert.WithinDuration(t, minSeen, *got.MinUserLastSeenAt, time.Second) +} + +// TestMongoStore_GetSubscription_ProjectionFields_Integration pins the field +// set that GetSubscription's projection must return: every Subscription field +// read by any handler call site. Same guard rationale as the GetRoom variant. +func TestMongoStore_GetSubscription_ProjectionFields_Integration(t *testing.T) { + ctx := context.Background() + db := setupMongo(t) + store := NewMongoStore(db) + + lastSeen := time.Now().UTC().Add(-30 * time.Minute).Truncate(time.Millisecond) + mustInsertSub(t, db, &model.Subscription{ + ID: "sproj", + User: model.SubscriptionUser{ID: "u9", Account: "carol", IsBot: true}, + RoomID: "rproj", + SiteID: "site-a", + Roles: []model.Role{model.RoleOwner, model.RoleMember}, + Alert: true, + ThreadUnread: []string{"t1", "t2"}, + LastSeenAt: &lastSeen, + }) + + got, err := store.GetSubscription(ctx, "carol", "rproj") + require.NoError(t, err) + assert.Equal(t, "u9", got.User.ID) + assert.Equal(t, "carol", got.User.Account) + assert.Equal(t, "rproj", got.RoomID) + assert.Equal(t, "site-a", got.SiteID) + assert.Equal(t, []model.Role{model.RoleOwner, model.RoleMember}, got.Roles) + assert.True(t, got.Alert) + assert.Equal(t, []string{"t1", "t2"}, got.ThreadUnread) + require.NotNil(t, got.LastSeenAt) + assert.WithinDuration(t, lastSeen, *got.LastSeenAt, time.Second) +} + func TestMongoStore_GetSubscriptionWithMembership_Integration(t *testing.T) { db := setupMongo(t) store := NewMongoStore(db) @@ -2336,9 +2424,9 @@ func TestMongoStore_ToggleSubscriptionMute(t *testing.T) { assert.Equal(t, "alice", got.User.Account) assert.Equal(t, "r1", got.RoomID) - persisted, err := store.GetSubscription(ctx, "alice", "r1") - require.NoError(t, err) - assert.True(t, persisted.Muted) + // muted is not projected by GetSubscription (no handler reads it from a read + // result), so verify persistence with a direct field read. + assert.True(t, subBoolField(t, db, "r1", "alice", "muted")) // muteUpdatedAt is stamped at the supplied instant (BSON ms precision) so the // origin doc shares the federated event's high-water mark. assert.Equal(t, ts1.UnixMilli(), subTimeField(t, db, "r1", "alice", "muteUpdatedAt").UnixMilli()) @@ -2371,6 +2459,22 @@ func subTimeField(t *testing.T, db *mongo.Database, roomID, account, field strin return dt.Time().UTC() } +// subBoolField reads a boolean field straight from the stored subscription +// document. Used to assert persistence of fields (muted, favorite) that +// GetSubscription deliberately does not project, so the read-back cannot rely +// on the projected struct. +func subBoolField(t *testing.T, db *mongo.Database, roomID, account, field string) bool { + t.Helper() + var doc bson.M + require.NoError(t, db.Collection("subscriptions"). + FindOne(context.Background(), bson.M{"roomId": roomID, "u.account": account}).Decode(&doc)) + v, ok := doc[field] + require.True(t, ok, "field %q missing on subscription", field) + b, ok := v.(bool) + require.True(t, ok, "field %q is %T, want bool", field, v) + return b +} + func TestMongoStore_ToggleSubscriptionFavorite(t *testing.T) { db := testutil.MongoDB(t, "room-svc-fav") store := NewMongoStore(db) @@ -2400,9 +2504,9 @@ func TestMongoStore_ToggleSubscriptionFavorite(t *testing.T) { assert.Equal(t, "alice", got.User.Account) assert.Equal(t, "r1", got.RoomID) - persisted, err := store.GetSubscription(ctx, "alice", "r1") - require.NoError(t, err) - assert.True(t, persisted.Favorite) + // favorite is not projected by GetSubscription (no handler reads it from a + // read result), so verify persistence with a direct field read. + assert.True(t, subBoolField(t, db, "r1", "alice", "favorite")) // favoriteUpdatedAt is stamped at the supplied instant so the origin doc // shares the federated event's high-water mark. assert.Equal(t, ts1.UnixMilli(), subTimeField(t, db, "r1", "alice", "favoriteUpdatedAt").UnixMilli()) diff --git a/room-service/mock_store_test.go b/room-service/mock_store_test.go index 5955585aa..16a84de64 100644 --- a/room-service/mock_store_test.go +++ b/room-service/mock_store_test.go @@ -57,6 +57,20 @@ func (mr *MockRoomStoreMockRecorder) ApplySubscriptionVisibility(ctx, roomID, re return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ApplySubscriptionVisibility", reflect.TypeOf((*MockRoomStore)(nil).ApplySubscriptionVisibility), ctx, roomID, restricted, externalAccess, ownerAccount, visibilityUpdatedAt) } +// CheckMembership mocks base method. +func (m *MockRoomStore) CheckMembership(ctx context.Context, account, roomID string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CheckMembership", ctx, account, roomID) + ret0, _ := ret[0].(error) + return ret0 +} + +// CheckMembership indicates an expected call of CheckMembership. +func (mr *MockRoomStoreMockRecorder) CheckMembership(ctx, account, roomID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckMembership", reflect.TypeOf((*MockRoomStore)(nil).CheckMembership), ctx, account, roomID) +} + // CountMembersAndOwners mocks base method. func (m *MockRoomStore) CountMembersAndOwners(ctx context.Context, roomID string) (*RoomCounts, error) { m.ctrl.T.Helper() diff --git a/room-service/store.go b/room-service/store.go index adca03570..e978daeac 100644 --- a/room-service/store.go +++ b/room-service/store.go @@ -64,6 +64,13 @@ type RoomStore interface { ListRoomsByIDs(ctx context.Context, ids []string) ([]model.Room, error) GetThreadUnreadSummary(ctx context.Context, account, siteID string) (*ThreadUnreadSummary, error) GetSubscription(ctx context.Context, account, roomID string) (*model.Subscription, error) + // CheckMembership verifies (account, roomID) has a subscription without + // decoding the document — an {_id:1}-projected existence check for the + // many call sites that only need the membership gate, not the sub's fields. + // Returns nil when subscribed, model.ErrSubscriptionNotFound (wrapped) when + // not, or a wrapped infra error otherwise. Same error contract as + // GetSubscription so callers branch identically on errors.Is. + CheckMembership(ctx context.Context, account, roomID string) error // ListMemberStatuses returns up to `limit` members of roomID, each // projected from the corresponding users document as {account, engName, // chineseName, statusIsShow, statusText}. Subscriptions whose user diff --git a/room-service/store_mongo.go b/room-service/store_mongo.go index 8baf95cb9..35ca7d9cf 100644 --- a/room-service/store_mongo.go +++ b/room-service/store_mongo.go @@ -238,9 +238,34 @@ func (s *MongoStore) InsertTeamsMeeting(ctx context.Context, record model.TeamsM return nil } +// roomReadProjection is the field set GetRoom returns — the union of every +// Room field read by a handler call site. The full Room doc is never needed on +// these read paths, and projecting trims the BSON decode (a top CPU consumer in +// profiling) plus the wire payload. Keep in sync with the Room field reads in +// handler.go; the projection-field integration test guards drift. +var roomReadProjection = bson.D{ + {Key: "_id", Value: 1}, {Key: "type", Value: 1}, {Key: "name", Value: 1}, + {Key: "userCount", Value: 1}, {Key: "appCount", Value: 1}, + {Key: "restricted", Value: 1}, {Key: "externalAccess", Value: 1}, + {Key: "lastMsgAt", Value: 1}, {Key: "minUserLastSeenAt", Value: 1}, +} + +// subscriptionReadProjection is the field set GetSubscription returns — the +// union of every Subscription field read by a handler call site. The fat +// Subscription doc (~30 fields incl. byte arrays and time pointers) is never +// needed here; projecting trims the reflection-heavy BSON decode. Keep in sync +// with the Subscription field reads in handler.go; the projection-field +// integration test guards drift. +var subscriptionReadProjection = bson.D{ + {Key: "_id", Value: 1}, {Key: "u", Value: 1}, {Key: "roomId", Value: 1}, + {Key: "siteId", Value: 1}, {Key: "roles", Value: 1}, {Key: "alert", Value: 1}, + {Key: "threadUnread", Value: 1}, {Key: "lastSeenAt", Value: 1}, +} + func (s *MongoStore) GetRoom(ctx context.Context, id string) (*model.Room, error) { var room model.Room - if err := s.rooms.FindOne(ctx, bson.M{"_id": id}).Decode(&room); err != nil { + opts := options.FindOne().SetProjection(roomReadProjection) + if err := s.rooms.FindOne(ctx, bson.M{"_id": id}, opts).Decode(&room); err != nil { return nil, fmt.Errorf("room %q not found: %w", id, err) } return &room, nil @@ -249,7 +274,8 @@ func (s *MongoStore) GetRoom(ctx context.Context, id string) (*model.Room, error func (s *MongoStore) GetSubscription(ctx context.Context, account, roomID string) (*model.Subscription, error) { var sub model.Subscription filter := bson.M{"u.account": account, "roomId": roomID} - if err := s.subscriptions.FindOne(ctx, filter).Decode(&sub); err != nil { + opts := options.FindOne().SetProjection(subscriptionReadProjection) + if err := s.subscriptions.FindOne(ctx, filter, opts).Decode(&sub); err != nil { if errors.Is(err, mongo.ErrNoDocuments) { return nil, fmt.Errorf("%q in room %q: %w", account, roomID, model.ErrSubscriptionNotFound) } @@ -258,6 +284,23 @@ func (s *MongoStore) GetSubscription(ctx context.Context, account, roomID string return &sub, nil } +// membershipExistsProjection returns only _id so the existence check decodes +// essentially nothing — the cheapest form of GetSubscription for call sites +// that use the result solely as a membership gate. +var membershipExistsProjection = bson.D{{Key: "_id", Value: 1}} + +func (s *MongoStore) CheckMembership(ctx context.Context, account, roomID string) error { + filter := bson.M{"u.account": account, "roomId": roomID} + opts := options.FindOne().SetProjection(membershipExistsProjection) + if err := s.subscriptions.FindOne(ctx, filter, opts).Err(); err != nil { + if errors.Is(err, mongo.ErrNoDocuments) { + return fmt.Errorf("%q in room %q: %w", account, roomID, model.ErrSubscriptionNotFound) + } + return fmt.Errorf("check membership for %q in room %q: %w", account, roomID, err) + } + return nil +} + // GetSubscriptionWithMembership loads the target subscription joined with their // individual and org membership sources. Used by the remove-member validation // flow to decide whether a user can leave or be removed individually.