MM-52600: [Shared Channels] Shared channels do not sync channel membership (#30976)

2025-06-15 10:07:56 +02:00
--- a/server/channels/store/storetest/channel_store.go
+++ b/server/channels/store/storetest/channel_store.go
@@ -73,6 +73,7 @@ func TestChannelStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore
 	t.Run("Save", func(t *testing.T) { testChannelStoreSave(t, rctx, ss) })
 	t.Run("SaveDirectChannel", func(t *testing.T) { testChannelStoreSaveDirectChannel(t, rctx, ss, s) })
 	t.Run("CreateDirectChannel", func(t *testing.T) { testChannelStoreCreateDirectChannel(t, rctx, ss) })
+	t.Run("GetMembersWithCursorPagination", func(t *testing.T) { testChannelStoreGetMembersWithCursorPagination(t, rctx, ss) })
 	t.Run("Update", func(t *testing.T) { testChannelStoreUpdate(t, rctx, ss) })
 	t.Run("GetChannelUnread", func(t *testing.T) { testGetChannelUnread(t, rctx, ss) })
 	t.Run("Get", func(t *testing.T) { testChannelStoreGet(t, rctx, ss, s) })
@@ -265,7 +266,7 @@ func testChannelStoreSaveDirectChannel(t *testing.T, rctx request.CTX, ss store.
 	_, nErr = ss.Channel().SaveDirectChannel(rctx, &o1, &m1, &m2)
 	require.NoError(t, nErr, "couldn't save direct channel", nErr)

-	members, nErr := ss.Channel().GetMembers(o1.Id, 0, 100)
+	members, nErr := ss.Channel().GetMembers(model.ChannelMembersGetOptions{ChannelID: o1.Id, Offset: 0, Limit: 100})
 	require.NoError(t, nErr)
 	require.Len(t, members, 2, "should have saved 2 members")

@@ -305,7 +306,7 @@ func testChannelStoreSaveDirectChannel(t *testing.T, rctx request.CTX, ss store.
 	_, nErr = ss.Channel().SaveDirectChannel(rctx, &o1, &m1, &m1)
 	require.NoError(t, nErr, "couldn't save direct channel", nErr)

-	members, nErr = ss.Channel().GetMembers(o1.Id, 0, 100)
+	members, nErr = ss.Channel().GetMembers(model.ChannelMembersGetOptions{ChannelID: o1.Id, Offset: 0, Limit: 100})
 	require.NoError(t, nErr)
 	require.Len(t, members, 1, "should have saved just 1 member")

@@ -341,11 +342,72 @@ func testChannelStoreCreateDirectChannel(t *testing.T, rctx request.CTX, ss stor
 		ss.Channel().PermanentDelete(rctx, c1.Id)
 	}()

-	members, nErr := ss.Channel().GetMembers(c1.Id, 0, 100)
+	members, nErr := ss.Channel().GetMembers(model.ChannelMembersGetOptions{ChannelID: c1.Id, Offset: 0, Limit: 100})
 	require.NoError(t, nErr)
 	require.Len(t, members, 2, "should have saved 2 members")
 }

+// testChannelStoreGetMembersWithCursorPagination tests the cursor-based pagination functionality
+// of the GetMembers method, using the UpdatedAfter parameter to return only members that were
+// updated after a specific timestamp.
+func testChannelStoreGetMembersWithCursorPagination(t *testing.T, rctx request.CTX, ss store.Store) {
+	// Create two users
+	u1 := &model.User{}
+	u1.Email = MakeEmail()
+	u1.Nickname = model.NewId()
+	_, err := ss.User().Save(rctx, u1)
+	require.NoError(t, err)
+	_, nErr := ss.Team().SaveMember(rctx, &model.TeamMember{TeamId: model.NewId(), UserId: u1.Id}, -1)
+	require.NoError(t, nErr)
+
+	u2 := &model.User{}
+	u2.Email = MakeEmail()
+	u2.Nickname = model.NewId()
+	_, err = ss.User().Save(rctx, u2)
+	require.NoError(t, err)
+	_, nErr = ss.Team().SaveMember(rctx, &model.TeamMember{TeamId: model.NewId(), UserId: u2.Id}, -1)
+	require.NoError(t, nErr)
+
+	// Create direct channel between the users
+	c1, nErr := ss.Channel().CreateDirectChannel(rctx, u1, u2)
+	require.NoError(t, nErr, "couldn't create direct channel", nErr)
+	defer func() {
+		ss.Channel().PermanentDeleteMembersByChannel(rctx, c1.Id)
+		ss.Channel().PermanentDelete(rctx, c1.Id)
+	}()
+
+	// First get all members
+	members, nErr := ss.Channel().GetMembers(model.ChannelMembersGetOptions{ChannelID: c1.Id, Offset: 0, Limit: 100})
+	require.NoError(t, nErr)
+	require.Len(t, members, 2, "should have saved 2 members")
+
+	// Ensure members have different LastUpdateAt values by updating one of them after a short delay
+	time.Sleep(1 * time.Millisecond)
+	member := members[0]
+	_, err = ss.Channel().UpdateMember(rctx, &member)
+	require.NoError(t, err)
+
+	// Get members again after the update
+	members, nErr = ss.Channel().GetMembers(model.ChannelMembersGetOptions{ChannelID: c1.Id, Offset: 0, Limit: 100})
+	require.NoError(t, nErr)
+	require.Len(t, members, 2, "should have 2 members")
+
+	// Find member with smaller LastUpdateAt
+	sort.Slice(members, func(i, j int) bool {
+		return members[i].LastUpdateAt < members[j].LastUpdateAt
+	})
+	updateTime := members[0].LastUpdateAt
+
+	// Test cursor-based pagination by querying for members updated after that timestamp
+	membersAfter, nErr := ss.Channel().GetMembers(model.ChannelMembersGetOptions{
+		ChannelID:    c1.Id,
+		UpdatedAfter: updateTime,
+		Limit:        100,
+	})
+	require.NoError(t, nErr)
+	require.Len(t, membersAfter, 1, "should have found only 1 member created after the timestamp")
+}
+
 func testChannelStoreUpdate(t *testing.T, rctx request.CTX, ss store.Store) {
 	o1 := model.Channel{}
 	o1.TeamId = model.NewId()
@@ -7867,7 +7929,7 @@ func testChannelStoreRemoveAllDeactivatedMembers(t *testing.T, rctx request.CTX,
 	require.NoError(t, err)

 	// Get all the channel members. Check there are 3.
-	d1, err := ss.Channel().GetMembers(c1.Id, 0, 1000)
+	d1, err := ss.Channel().GetMembers(model.ChannelMembersGetOptions{ChannelID: c1.Id, Offset: 0, Limit: 1000})
 	assert.NoError(t, err)
 	assert.Len(t, d1, 3)

@@ -7887,7 +7949,7 @@ func testChannelStoreRemoveAllDeactivatedMembers(t *testing.T, rctx request.CTX,
 	assert.NoError(t, ss.Channel().RemoveAllDeactivatedMembers(rctx, c1.Id))

 	// Get all the channel members. Check there is now only 1: m3.
-	d2, err := ss.Channel().GetMembers(c1.Id, 0, 1000)
+	d2, err := ss.Channel().GetMembers(model.ChannelMembersGetOptions{ChannelID: c1.Id, Offset: 0, Limit: 1000})
 	assert.NoError(t, err)
 	assert.Len(t, d2, 1)
 	assert.Equal(t, u3.Id, d2[0].UserId)
--- a/server/channels/store/storetest/group_store.go
+++ b/server/channels/store/storetest/group_store.go
@@ -5340,7 +5340,7 @@ func groupTestpUpdateMembersRoleChannel(t *testing.T, rctx request.CTX, ss store
 			}
 			assert.ElementsMatch(t, tt.expectedUpdatedUsers, updatedUserIDs)

-			members, err := ss.Channel().GetMembers(channel.Id, 0, 100)
+			members, err := ss.Channel().GetMembers(model.ChannelMembersGetOptions{ChannelID: channel.Id, Offset: 0, Limit: 100})
 			require.NoError(t, err)
 			assert.GreaterOrEqual(t, len(members), 4) // sanity check for channel membership

--- a/server/channels/store/storetest/mocks/ChannelStore.go
+++ b/server/channels/store/storetest/mocks/ChannelStore.go
@@ -1580,9 +1580,9 @@ func (_m *ChannelStore) GetMemberLastViewedAt(ctx context.Context, channelID str
 	return r0, r1
 }

-// GetMembers provides a mock function with given fields: channelID, offset, limit
-func (_m *ChannelStore) GetMembers(channelID string, offset int, limit int) (model.ChannelMembers, error) {
-	ret := _m.Called(channelID, offset, limit)
+// GetMembers provides a mock function with given fields: opts
+func (_m *ChannelStore) GetMembers(opts model.ChannelMembersGetOptions) (model.ChannelMembers, error) {
+	ret := _m.Called(opts)

 	if len(ret) == 0 {
 		panic("no return value specified for GetMembers")
@@ -1590,19 +1590,19 @@ func (_m *ChannelStore) GetMembers(channelID string, offset int, limit int) (mod

 	var r0 model.ChannelMembers
 	var r1 error
-	if rf, ok := ret.Get(0).(func(string, int, int) (model.ChannelMembers, error)); ok {
-		return rf(channelID, offset, limit)
+	if rf, ok := ret.Get(0).(func(model.ChannelMembersGetOptions) (model.ChannelMembers, error)); ok {
+		return rf(opts)
 	}
-	if rf, ok := ret.Get(0).(func(string, int, int) model.ChannelMembers); ok {
-		r0 = rf(channelID, offset, limit)
+	if rf, ok := ret.Get(0).(func(model.ChannelMembersGetOptions) model.ChannelMembers); ok {
+		r0 = rf(opts)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).(model.ChannelMembers)
 		}
 	}

-	if rf, ok := ret.Get(1).(func(string, int, int) error); ok {
-		r1 = rf(channelID, offset, limit)
+	if rf, ok := ret.Get(1).(func(model.ChannelMembersGetOptions) error); ok {
+		r1 = rf(opts)
 	} else {
 		r1 = ret.Error(1)
 	}
--- a/server/channels/store/storetest/mocks/SharedChannelStore.go
+++ b/server/channels/store/storetest/mocks/SharedChannelStore.go
@@ -368,6 +368,36 @@ func (_m *SharedChannelStore) GetSingleUser(userID string, channelID string, rem
 	return r0, r1
 }

+// GetUserChanges provides a mock function with given fields: userID, channelID, afterTime
+func (_m *SharedChannelStore) GetUserChanges(userID string, channelID string, afterTime int64) ([]*model.SharedChannelUser, error) {
+	ret := _m.Called(userID, channelID, afterTime)
+
+	if len(ret) == 0 {
+		panic("no return value specified for GetUserChanges")
+	}
+
+	var r0 []*model.SharedChannelUser
+	var r1 error
+	if rf, ok := ret.Get(0).(func(string, string, int64) ([]*model.SharedChannelUser, error)); ok {
+		return rf(userID, channelID, afterTime)
+	}
+	if rf, ok := ret.Get(0).(func(string, string, int64) []*model.SharedChannelUser); ok {
+		r0 = rf(userID, channelID, afterTime)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).([]*model.SharedChannelUser)
+		}
+	}
+
+	if rf, ok := ret.Get(1).(func(string, string, int64) error); ok {
+		r1 = rf(userID, channelID, afterTime)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // GetUsersForSync provides a mock function with given fields: filter
 func (_m *SharedChannelStore) GetUsersForSync(filter model.GetUsersForSyncFilter) ([]*model.User, error) {
 	ret := _m.Called(filter)
@@ -700,6 +730,42 @@ func (_m *SharedChannelStore) UpdateRemoteCursor(id string, cursor model.GetPost
 	return r0
 }

+// UpdateRemoteMembershipCursor provides a mock function with given fields: id, syncTime
+func (_m *SharedChannelStore) UpdateRemoteMembershipCursor(id string, syncTime int64) error {
+	ret := _m.Called(id, syncTime)
+
+	if len(ret) == 0 {
+		panic("no return value specified for UpdateRemoteMembershipCursor")
+	}
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(string, int64) error); ok {
+		r0 = rf(id, syncTime)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
+// UpdateUserLastMembershipSyncAt provides a mock function with given fields: userID, channelID, remoteID, syncTime
+func (_m *SharedChannelStore) UpdateUserLastMembershipSyncAt(userID string, channelID string, remoteID string, syncTime int64) error {
+	ret := _m.Called(userID, channelID, remoteID, syncTime)
+
+	if len(ret) == 0 {
+		panic("no return value specified for UpdateUserLastMembershipSyncAt")
+	}
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(string, string, string, int64) error); ok {
+		r0 = rf(userID, channelID, remoteID, syncTime)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
 // UpdateUserLastSyncAt provides a mock function with given fields: userID, channelID, remoteID
 func (_m *SharedChannelStore) UpdateUserLastSyncAt(userID string, channelID string, remoteID string) error {
 	ret := _m.Called(userID, channelID, remoteID)