From fa1c77d9b04e0eb7efbcb78fb0994005721f3ac2 Mon Sep 17 00:00:00 2001 From: catalintomai <56169943+catalintomai@users.noreply.github.com> Date: Sun, 15 Jun 2025 10:07:56 +0200 Subject: [PATCH] MM-52600: [Shared Channels] Shared channels do not sync channel membership (#30976) --- server/channels/app/channel.go | 29 +- server/channels/app/channel_test.go | 12 +- .../platform/shared_channel_service_iface.go | 5 + ...l_membership_sync_self_referential_test.go | 1957 +++++++++++++++++ .../app/shared_channel_service_iface.go | 7 + ...hannel_sync_self_referential_utils_test.go | 94 +- server/channels/app/shared_channel_test.go | 1 + server/channels/db/migrations/migrations.list | 4 + ...erssyncat_to_sharedchannelremotes.down.sql | 29 + ...mberssyncat_to_sharedchannelremotes.up.sql | 29 + ...erssyncat_to_sharedchannelremotes.down.sql | 2 + ...mberssyncat_to_sharedchannelremotes.up.sql | 2 + .../channels/store/retrylayer/retrylayer.go | 67 +- .../channels/store/sqlstore/channel_store.go | 30 +- .../store/sqlstore/shared_channel_store.go | 23 +- .../shared_channel_store_membership.go | 66 + server/channels/store/store.go | 5 +- .../channels/store/storetest/channel_store.go | 72 +- .../channels/store/storetest/group_store.go | 2 +- .../store/storetest/mocks/ChannelStore.go | 18 +- .../storetest/mocks/SharedChannelStore.go | 66 + .../channels/store/timerlayer/timerlayer.go | 52 +- .../services/remotecluster/service.go | 2 +- .../services/sharedchannel/channelinvite.go | 33 + .../sharedchannel/channelinvite_test.go | 17 + .../services/sharedchannel/membership.go | 409 ++++ .../services/sharedchannel/membership_recv.go | 213 ++ .../sharedchannel/mock_AppIface_test.go | 20 + .../services/sharedchannel/service.go | 38 +- .../services/sharedchannel/service_api.go | 38 + .../services/sharedchannel/sync_recv.go | 28 +- .../services/sharedchannel/sync_send.go | 22 +- .../sharedchannel/sync_send_remote.go | 1 + server/public/model/channel.go | 12 + server/public/model/config.go | 11 +- server/public/model/feature_flags.go | 4 + server/public/model/shared_channel.go | 36 +- 37 files changed, 3371 insertions(+), 85 deletions(-) create mode 100644 server/channels/app/shared_channel_membership_sync_self_referential_test.go create mode 100644 server/channels/db/migrations/mysql/000140_add_lastmemberssyncat_to_sharedchannelremotes.down.sql create mode 100644 server/channels/db/migrations/mysql/000140_add_lastmemberssyncat_to_sharedchannelremotes.up.sql create mode 100644 server/channels/db/migrations/postgres/000140_add_lastmemberssyncat_to_sharedchannelremotes.down.sql create mode 100644 server/channels/db/migrations/postgres/000140_add_lastmemberssyncat_to_sharedchannelremotes.up.sql create mode 100644 server/channels/store/sqlstore/shared_channel_store_membership.go create mode 100644 server/platform/services/sharedchannel/membership.go create mode 100644 server/platform/services/sharedchannel/membership_recv.go diff --git a/server/channels/app/channel.go b/server/channels/app/channel.go index 328e1c43c40..5ef44c817ee 100644 --- a/server/channels/app/channel.go +++ b/server/channels/app/channel.go @@ -1711,6 +1711,13 @@ func (a *App) addUserToChannel(c request.CTX, user *model.User, channel *model.C a.Srv().Platform().InvalidateChannelCacheForUser(user.Id) a.invalidateCacheForChannelMembers(channel.Id) + // Synchronize membership change for shared channels + if channel.IsShared() { + if scs := a.Srv().Platform().GetSharedChannelService(); scs != nil { + scs.HandleMembershipChange(channel.Id, user.Id, true, user.GetRemoteID()) + } + } + return newMember, nil } @@ -2236,7 +2243,12 @@ func (s *Server) getChannelMemberLastViewedAt(c request.CTX, channelID string, u } func (a *App) GetChannelMembersPage(c request.CTX, channelID string, page, perPage int) (model.ChannelMembers, *model.AppError) { - channelMembers, err := a.Srv().Store().Channel().GetMembers(channelID, page*perPage, perPage) + opts := model.ChannelMembersGetOptions{ + ChannelID: channelID, + Offset: page * perPage, + Limit: perPage, + } + channelMembers, err := a.Srv().Store().Channel().GetMembers(opts) if err != nil { return nil, model.NewAppError("GetChannelMembersPage", "app.channel.get_members.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -2740,6 +2752,14 @@ func (a *App) removeUserFromChannel(c request.CTX, userIDToRemove string, remove userMsg.Add("remover_id", removerUserId) a.Publish(userMsg) + // Synchronize membership change for shared channels + if channel.IsShared() { + // isAdd=false, empty remoteId means locally initiated + if scs := a.Srv().Platform().GetSharedChannelService(); scs != nil { + scs.HandleMembershipChange(channel.Id, userIDToRemove, false, "") + } + } + return nil } @@ -3639,7 +3659,12 @@ func (a *App) forEachChannelMember(c request.CTX, channelID string, f func(model page := 0 for { - channelMembers, err := a.Srv().Store().Channel().GetMembers(channelID, page*perPage, perPage) + opts := model.ChannelMembersGetOptions{ + ChannelID: channelID, + Offset: page * perPage, + Limit: perPage, + } + channelMembers, err := a.Srv().Store().Channel().GetMembers(opts) if err != nil { return err } diff --git a/server/channels/app/channel_test.go b/server/channels/app/channel_test.go index bb2c1fc0c9c..75a4880756d 100644 --- a/server/channels/app/channel_test.go +++ b/server/channels/app/channel_test.go @@ -2513,8 +2513,16 @@ func TestClearChannelMembersCache(t *testing.T) { ChannelId: "1", }) } - mockChannelStore.On("GetMembers", "channelID", 0, 100).Return(cms, nil) - mockChannelStore.On("GetMembers", "channelID", 100, 100).Return(model.ChannelMembers{ + mockChannelStore.On("GetMembers", model.ChannelMembersGetOptions{ + ChannelID: "channelID", + Offset: 0, + Limit: 100, + }).Return(cms, nil) + mockChannelStore.On("GetMembers", model.ChannelMembersGetOptions{ + ChannelID: "channelID", + Offset: 100, + Limit: 100, + }).Return(model.ChannelMembers{ model.ChannelMember{ ChannelId: "1", }, diff --git a/server/channels/app/platform/shared_channel_service_iface.go b/server/channels/app/platform/shared_channel_service_iface.go index 63ee643c241..2adc6a1c599 100644 --- a/server/channels/app/platform/shared_channel_service_iface.go +++ b/server/channels/app/platform/shared_channel_service_iface.go @@ -23,6 +23,7 @@ type SharedChannelServiceIFace interface { CheckChannelNotShared(channelID string) error CheckChannelIsShared(channelID string) error CheckCanInviteToSharedChannel(channelId string) error + HandleMembershipChange(channelID, userID string, isAdd bool, remoteID string) } type MockOptionSharedChannelService func(service *mockSharedChannelService) @@ -77,3 +78,7 @@ func (mrcs *mockSharedChannelService) SendChannelInvite(channel *model.Channel, func (mrcs *mockSharedChannelService) NumInvitations() int { return mrcs.numInvitations } + +func (mrcs *mockSharedChannelService) HandleMembershipChange(channelID, userID string, isAdd bool, remoteID string) { + // This is a mock implementation - it doesn't need to do anything +} diff --git a/server/channels/app/shared_channel_membership_sync_self_referential_test.go b/server/channels/app/shared_channel_membership_sync_self_referential_test.go new file mode 100644 index 00000000000..c099a6a3379 --- /dev/null +++ b/server/channels/app/shared_channel_membership_sync_self_referential_test.go @@ -0,0 +1,1957 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "os" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/v8/platform/services/remotecluster" + "github.com/mattermost/mattermost/server/v8/platform/services/sharedchannel" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) { + th := setupSharedChannels(t).InitBasic() + defer th.TearDown() + + ss := th.App.Srv().Store() + + // Get the shared channel service and cast to concrete type to access SyncAllChannelMembers + scsInterface := th.App.Srv().GetSharedChannelSyncService() + service, ok := scsInterface.(*sharedchannel.Service) + require.True(t, ok, "Expected sharedchannel.Service concrete type") + + // Force the service to be active + err := service.Start() + require.NoError(t, err) + + // Wait for service to be fully active + require.Eventually(t, func() bool { + return service.Active() + }, 5*time.Second, 100*time.Millisecond, "SharedChannelService should be active") + + // Also ensure the remote cluster service is running so callbacks work + rcService := th.App.Srv().GetRemoteClusterService() + if rcService != nil { + _ = rcService.Start() + + // Force the service to be active in test environment + if rc, ok := rcService.(*remotecluster.Service); ok { + rc.SetActive(true) + } + + // Wait for remote cluster service to be active + require.Eventually(t, func() bool { + return rcService.Active() + }, 5*time.Second, 100*time.Millisecond, "RemoteClusterService should be active") + } + + t.Run("Test 1: Automatic sync on membership changes", func(t *testing.T) { + // This test verifies that membership sync happens automatically when users are added or removed from a shared channel. + // The sync is triggered by HandleMembershipChange which is called automatically by AddUserToChannel and RemoveUserFromChannel. + // The test ensures that sync messages are sent asynchronously after a minimum delay for both add and remove operations. + EnsureCleanState(t, th, ss) + // Track sync messages received + var syncMessageCount int32 + var syncHandler *SelfReferentialSyncHandler + + // Create a test HTTP server that acts as the "remote" cluster + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&syncMessageCount, 1) + if syncHandler != nil { + syncHandler.HandleRequest(w, r) + } else { + writeOKResponse(w) + } + })) + defer testServer.Close() + + // Create a shared channel + channel := th.CreateChannel(th.Context, th.BasicTeam) + sc := &model.SharedChannel{ + ChannelId: channel.Id, + TeamId: th.BasicTeam.Id, + Home: true, + ShareName: "auto-sync-test", + CreatorId: th.BasicUser.Id, + RemoteId: "", + } + _, shareErr := th.App.ShareChannel(th.Context, sc) + require.NoError(t, shareErr) + + // Create a self-referential remote cluster + selfCluster := &model.RemoteCluster{ + RemoteId: model.NewId(), + Name: "self-cluster", + SiteURL: testServer.URL, + CreateAt: model.GetMillis(), + LastPingAt: model.GetMillis(), + Token: model.NewId(), + CreatorId: th.BasicUser.Id, + RemoteToken: model.NewId(), + } + selfCluster, err = ss.RemoteCluster().Save(selfCluster) + require.NoError(t, err) + + // Initialize sync handler + syncHandler = NewSelfReferentialSyncHandler(t, service, selfCluster) + + // Share the channel with our self-referential cluster + scr := &model.SharedChannelRemote{ + Id: model.NewId(), + ChannelId: channel.Id, + CreatorId: th.BasicUser.Id, + RemoteId: selfCluster.RemoteId, + IsInviteAccepted: true, + IsInviteConfirmed: true, + LastMembersSyncAt: 0, + } + _, err = ss.SharedChannel().SaveRemote(scr) + require.NoError(t, err) + + // Refresh the channel object to get the updated Shared field + channel, appErr := th.App.GetChannel(th.Context, channel.Id) + require.Nil(t, appErr) + require.True(t, channel.IsShared(), "Channel should be marked as shared") + + // Create a user and add to team + user := th.CreateUser() + _, _, appErr = th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, user.Id, th.BasicUser.Id) + require.Nil(t, appErr) + + // Add user to channel - this triggers HandleMembershipChange automatically + _, appErr = th.App.AddUserToChannel(th.Context, user, channel, false) + require.Nil(t, appErr) + + // Wait for the user to be locally added before checking for sync + require.Eventually(t, func() bool { + _, memberErr := ss.Channel().GetMember(context.Background(), channel.Id, user.Id) + return memberErr == nil + }, 5*time.Second, 100*time.Millisecond, "User should be locally added to channel") + + // Wait for async sync with more generous timeout (minimum delay is 2 seconds + async task processing) + require.Eventually(t, func() bool { + count := atomic.LoadInt32(&syncMessageCount) + return count > 0 + }, 15*time.Second, 200*time.Millisecond, "Should have received at least one sync message via automatic sync") + + // Wait for async task queue to be processed and sync to complete + require.Eventually(t, func() bool { + return !service.HasPendingTasksForTesting() + }, 10*time.Second, 200*time.Millisecond, "All async sync tasks should be completed") + + // Verify the user is a member at the receiver end + member, memberErr := ss.Channel().GetMember(context.Background(), channel.Id, user.Id) + require.NoError(t, memberErr) + require.Equal(t, user.Id, member.UserId) + + // Reset sync counter and wait for background tasks to settle + var initialCount int32 + require.Eventually(t, func() bool { + initialCount = atomic.LoadInt32(&syncMessageCount) + return !service.HasPendingTasksForTesting() + }, 5*time.Second, 100*time.Millisecond, "Background tasks should settle before removal test") + + // Remove user from channel - this should also trigger automatic sync + appErr = th.App.RemoveUserFromChannel(th.Context, user.Id, th.BasicUser.Id, channel) + require.Nil(t, appErr) + + // Wait for removal sync with increased timeout + require.Eventually(t, func() bool { + count := atomic.LoadInt32(&syncMessageCount) + return count > initialCount + }, 20*time.Second, 200*time.Millisecond, "Should have received sync message for user removal") + + // Wait for async task queue to be processed after removal + require.Eventually(t, func() bool { + return !service.HasPendingTasksForTesting() + }, 15*time.Second, 200*time.Millisecond, "All async removal tasks should be completed") + + // Wait for the removal to be processed with extended timeout + require.Eventually(t, func() bool { + _, err = ss.Channel().GetMember(context.Background(), channel.Id, user.Id) + return err != nil + }, 30*time.Second, 300*time.Millisecond, "User should not be a member after removal") + }) + t.Run("Test 2: Batch membership sync with user type filtering", func(t *testing.T) { + EnsureCleanState(t, th, ss) + // This test verifies batch sync of multiple members and proper filtering of user types + var batchedUserIDs [][]string + var mu sync.Mutex + var selfCluster *model.RemoteCluster + var syncCompleted atomic.Bool + + // Placeholder for sync handler - will be created after selfCluster is initialized + var syncHandler *SelfReferentialSyncHandler + + // Create test HTTP server + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if syncHandler != nil { + syncHandler.HandleRequest(w, r) + } else { + writeOKResponse(w) + } + })) + defer testServer.Close() + + // Create channel but DON'T share it yet + channel := th.CreateChannel(th.Context, th.BasicTeam) + + // Create self-referential remote cluster + selfCluster = &model.RemoteCluster{ + RemoteId: model.NewId(), + Name: "self-cluster-batch", + SiteURL: testServer.URL, + CreateAt: model.GetMillis(), + LastPingAt: model.GetMillis(), + Token: model.NewId(), + CreatorId: th.BasicUser.Id, + RemoteToken: model.NewId(), + } + selfCluster, err = ss.RemoteCluster().Save(selfCluster) + require.NoError(t, err) + + batchSize := service.GetMemberSyncBatchSize() + + // First, add multiple users to the channel BEFORE sharing + // Include different user types to test filtering + numRegularUsers := (batchSize * 2) + 5 // Back to original + regularUserIDs := make([]string, numRegularUsers) + for i := 0; i < numRegularUsers; i++ { + user := th.CreateUser() + regularUserIDs[i] = user.Id + _, _, appErr := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, user.Id, th.BasicUser.Id) + require.Nil(t, appErr) + _, appErr = th.App.AddUserToChannel(th.Context, user, channel, false) + require.Nil(t, appErr) + } + + // Add users that should be synced (including bots and system admins) + // Add a bot + bot := th.CreateBot() + botUser, appErr := th.App.GetUser(bot.UserId) + require.Nil(t, appErr) + _, _, appErr = th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, bot.UserId, th.BasicUser.Id) + require.Nil(t, appErr) + _, appErr = th.App.AddUserToChannel(th.Context, botUser, channel, false) + require.Nil(t, appErr) + + // Add a system admin + systemAdmin := th.CreateUser() + _, _, appErr = th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, systemAdmin.Id, th.BasicUser.Id) + require.Nil(t, appErr) + _, appErr = th.App.UpdateUserRoles(th.Context, systemAdmin.Id, model.SystemAdminRoleId+" "+model.SystemUserRoleId, false) + require.Nil(t, appErr) + _, appErr = th.App.AddUserToChannel(th.Context, systemAdmin, channel, false) + require.Nil(t, appErr) + + // Add a guest user (should be synced) + guest := th.CreateGuest() + _, _, appErr = th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, guest.Id, th.BasicUser.Id) + require.Nil(t, appErr) + _, appErr = th.App.AddUserToChannel(th.Context, guest, channel, false) + require.Nil(t, appErr) + + sc := &model.SharedChannel{ + ChannelId: channel.Id, + TeamId: th.BasicTeam.Id, + Home: true, + ShareName: "batch-sync-test", + CreatorId: th.BasicUser.Id, + RemoteId: "", + } + _, err = th.App.ShareChannel(th.Context, sc) + require.NoError(t, err) + + // Share channel with self + scr := &model.SharedChannelRemote{ + Id: model.NewId(), + ChannelId: channel.Id, + CreatorId: th.BasicUser.Id, + RemoteId: selfCluster.RemoteId, + IsInviteAccepted: true, + IsInviteConfirmed: true, + LastMembersSyncAt: 0, + } + _, err = ss.SharedChannel().SaveRemote(scr) + require.NoError(t, err) + + // Initialize sync handler with callbacks + syncHandler = NewSelfReferentialSyncHandler(t, service, selfCluster) + + // Track when all expected batches are received + // Should include regular users + guest + BasicUser + bot + system admin + expectedTotal := numRegularUsers + 1 + 1 + 1 + 1 // regular users + guest + BasicUser + bot + system admin + expectedBatches := (expectedTotal + batchSize - 1) / batchSize + + syncHandler.OnBatchSync = func(userIds []string, messageNumber int32) { + mu.Lock() + batchedUserIDs = append(batchedUserIDs, userIds) + // Check if we've received all expected batches + if len(batchedUserIDs) >= expectedBatches { + syncCompleted.Store(true) + } + mu.Unlock() + } + + err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) + require.NoError(t, err) + + // Wait for batch messages to be received with more robust checking + require.Eventually(t, func() bool { + return syncCompleted.Load() + }, 30*time.Second, 200*time.Millisecond, fmt.Sprintf("Should receive %d batch sync messages", expectedBatches)) + + // Wait for all async processing to complete + require.Eventually(t, func() bool { + return !service.HasPendingTasksForTesting() + }, 15*time.Second, 200*time.Millisecond, "All async tasks should be completed") + + // Verify cursor was updated - wait longer for batch processing + require.Eventually(t, func() bool { + updatedScr, getErr := ss.SharedChannel().GetRemoteByIds(channel.Id, selfCluster.RemoteId) + return getErr == nil && updatedScr.LastMembersSyncAt > 0 + }, 20*time.Second, 200*time.Millisecond, "Cursor should be updated after batch sync") + + // Verify sync messages were sent + count := syncHandler.GetSyncMessageCount() + assert.Greater(t, count, int32(0), "Should have received sync messages") + + // Check batch contents with proper locking + mu.Lock() + totalSynced := 0 + allSyncedUserIDs := make(map[string]bool) + actualBatches := len(batchedUserIDs) + for _, batch := range batchedUserIDs { + totalSynced += len(batch) + for _, userID := range batch { + allSyncedUserIDs[userID] = true + } + } + mu.Unlock() + + // Verify exact batch count + assert.Equal(t, expectedBatches, actualBatches, fmt.Sprintf("Should have exactly %d batches with batch size %d", expectedBatches, batchSize)) + + // Verify total synced users + assert.Equal(t, expectedTotal, totalSynced, "All users including bots and system admins should be synced") + + // Verify that bot and system admin WERE synced + assert.Contains(t, allSyncedUserIDs, bot.UserId, "Bot should be synced") + assert.Contains(t, allSyncedUserIDs, systemAdmin.Id, "System admin should be synced") + + // Verify that guest WAS synced + assert.Contains(t, allSyncedUserIDs, guest.Id, "Guest user should be synced") + + // Verify that regular users were synced + for _, regularUserID := range regularUserIDs { + assert.Contains(t, allSyncedUserIDs, regularUserID, "Regular user should be synced") + } + }) + t.Run("Test 3: Cursor management", func(t *testing.T) { + // This test verifies incremental sync using cursor timestamps. + // The cursor (LastMembersSyncAt) tracks the last sync time to ensure only new/modified + // memberships are synced on subsequent calls, avoiding redundant syncs. + // We test that: + // 1. Initial sync includes all existing members and updates the cursor + // 2. Subsequent syncs only include members added after the cursor timestamp + // 3. Previously synced members are not re-synced (incremental behavior) + EnsureCleanState(t, th, ss) + var syncedInFirstCall []string + var syncedInSecondCall []string + var mu sync.Mutex + var selfCluster *model.RemoteCluster + var svc *sharedchannel.Service + var syncHandler *SelfReferentialSyncHandler + + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if syncHandler != nil { + syncHandler.HandleRequest(w, r) + } else { + writeOKResponse(w) + } + })) + defer testServer.Close() + + // Create and share channel + channel := th.CreateChannel(th.Context, th.BasicTeam) + sc := &model.SharedChannel{ + ChannelId: channel.Id, + TeamId: th.BasicTeam.Id, + Home: true, + ShareName: "cursor-test", + CreatorId: th.BasicUser.Id, + RemoteId: "", + } + _, shareErr := th.App.ShareChannel(th.Context, sc) + require.NoError(t, shareErr) + + // Create self-referential remote cluster + selfCluster = &model.RemoteCluster{ + RemoteId: model.NewId(), + Name: "self-cluster-cursor", + SiteURL: testServer.URL, + CreateAt: model.GetMillis(), + LastPingAt: model.GetMillis(), + Token: model.NewId(), + CreatorId: th.BasicUser.Id, + RemoteToken: model.NewId(), + } + selfCluster, err = ss.RemoteCluster().Save(selfCluster) + require.NoError(t, err) + + // Share channel with self + scr := &model.SharedChannelRemote{ + Id: model.NewId(), + ChannelId: channel.Id, + CreatorId: th.BasicUser.Id, + RemoteId: selfCluster.RemoteId, + IsInviteAccepted: true, + IsInviteConfirmed: true, + LastMembersSyncAt: 0, + } + scr, err = ss.SharedChannel().SaveRemote(scr) + require.NoError(t, err) + + // Refresh the channel object to get the updated Shared field + channel, appErr := th.App.GetChannel(th.Context, channel.Id) + require.Nil(t, appErr) + require.True(t, channel.IsShared(), "Channel should be marked as shared") + + // Add first batch of users + user1 := th.CreateUser() + user2 := th.CreateUser() + + _, _, appErr = th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, user1.Id, th.BasicUser.Id) + require.Nil(t, appErr) + _, appErr = th.App.AddUserToChannel(th.Context, user1, channel, false) + require.Nil(t, appErr) + + _, _, appErr = th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, user2.Id, th.BasicUser.Id) + require.Nil(t, appErr) + _, appErr = th.App.AddUserToChannel(th.Context, user2, channel, false) + require.Nil(t, appErr) + + // Get the shared channel service + scsInterface := th.App.Srv().GetSharedChannelSyncService() + var ok bool + svc, ok = scsInterface.(*sharedchannel.Service) + require.True(t, ok, "Expected sharedchannel.Service concrete type") + + // Force the service to be active + err = svc.Start() + require.NoError(t, err) + + // Create sync handler with callbacks + syncHandler = NewSelfReferentialSyncHandler(t, svc, selfCluster) + syncHandler.OnIndividualSync = func(userId string, messageNumber int32) { + mu.Lock() + defer mu.Unlock() + if messageNumber <= 2 { // First sync + syncedInFirstCall = append(syncedInFirstCall, userId) + } else { // Second sync + syncedInSecondCall = append(syncedInSecondCall, userId) + } + } + + // First sync + err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) + require.NoError(t, err) + + // Wait for first sync to complete + require.Eventually(t, func() bool { + mu.Lock() + defer mu.Unlock() + return len(syncedInFirstCall) >= 2 + }, 10*time.Second, 100*time.Millisecond, "First sync should complete with initial users") + + // Wait for cursor to be updated after first sync + var firstSyncCursor int64 + require.Eventually(t, func() bool { + scr, err = ss.SharedChannel().GetRemoteByIds(channel.Id, selfCluster.RemoteId) + if err != nil { + return false + } + firstSyncCursor = scr.LastMembersSyncAt + return firstSyncCursor > 0 + }, 10*time.Second, 100*time.Millisecond, "Cursor should be updated after first sync") + + // Add another user after cursor update + user3 := th.CreateUser() + _, _, appErr = th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, user3.Id, th.BasicUser.Id) + require.Nil(t, appErr) + _, appErr = th.App.AddUserToChannel(th.Context, user3, channel, false) + require.Nil(t, appErr) + + // Second sync - should only sync user3 + err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) + require.NoError(t, err) + + // Wait for second sync to complete + require.Eventually(t, func() bool { + mu.Lock() + defer mu.Unlock() + return len(syncedInSecondCall) >= 1 + }, 10*time.Second, 100*time.Millisecond, "Second sync should complete with new user") + + // Wait for cursor to be updated after second sync + var secondSyncCursor int64 + require.Eventually(t, func() bool { + scr, err = ss.SharedChannel().GetRemoteByIds(channel.Id, selfCluster.RemoteId) + if err != nil { + return false + } + secondSyncCursor = scr.LastMembersSyncAt + return secondSyncCursor > firstSyncCursor + }, 10*time.Second, 100*time.Millisecond, "Cursor should advance after second sync") + + // Verify incremental sync + assert.GreaterOrEqual(t, len(syncedInFirstCall), 2, "First sync should include initial users") + assert.Contains(t, syncedInSecondCall, user3.Id, "Second sync should include only new user") + assert.NotContains(t, syncedInSecondCall, user1.Id, "Second sync should not re-sync existing users") + assert.NotContains(t, syncedInSecondCall, user2.Id, "Second sync should not re-sync existing users") + }) + t.Run("Test 4: Sync failure and recovery", func(t *testing.T) { + // This test verifies that membership sync handles remote server failures gracefully + // and successfully syncs members once the remote server recovers. + // We test that: + // 1. Sync attempts are made even when the remote server returns errors + // 2. No members are synced during failure mode + // 3. Once the server recovers, sync completes successfully + EnsureCleanState(t, th, ss) + var syncAttempts int32 + var failureMode atomic.Bool + failureMode.Store(true) + var successfulSyncs []string + var selfCluster *model.RemoteCluster + var syncHandler *SelfReferentialSyncHandler + + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/v4/remotecluster/msg" { + atomic.AddInt32(&syncAttempts, 1) + + if failureMode.Load() { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":"simulated failure"}`)) + return + } + + if syncHandler != nil { + syncHandler.HandleRequest(w, r) + } else { + writeOKResponse(w) + } + } else { + writeOKResponse(w) + } + })) + defer testServer.Close() + + // Create and share channel + channel := th.CreateChannel(th.Context, th.BasicTeam) + sc := &model.SharedChannel{ + ChannelId: channel.Id, + TeamId: th.BasicTeam.Id, + Home: true, + ShareName: "failure-recovery-test", + CreatorId: th.BasicUser.Id, + RemoteId: "", + } + _, shareErr := th.App.ShareChannel(th.Context, sc) + require.NoError(t, shareErr) + + // Create self-referential remote cluster + selfCluster = &model.RemoteCluster{ + RemoteId: model.NewId(), + Name: "self-cluster-failure", + SiteURL: testServer.URL, + CreateAt: model.GetMillis(), + LastPingAt: model.GetMillis(), + Token: model.NewId(), + CreatorId: th.BasicUser.Id, + RemoteToken: model.NewId(), + } + selfCluster, err = ss.RemoteCluster().Save(selfCluster) + require.NoError(t, err) + + // Share channel with self + scr := &model.SharedChannelRemote{ + Id: model.NewId(), + ChannelId: channel.Id, + CreatorId: th.BasicUser.Id, + RemoteId: selfCluster.RemoteId, + IsInviteAccepted: true, + IsInviteConfirmed: true, + LastMembersSyncAt: 0, + } + _, err = ss.SharedChannel().SaveRemote(scr) + require.NoError(t, err) + + // Initialize sync handler with callbacks + syncHandler = NewSelfReferentialSyncHandler(t, service, selfCluster) + syncHandler.OnBatchSync = func(userIds []string, messageNumber int32) { + successfulSyncs = append(successfulSyncs, userIds...) + } + syncHandler.OnIndividualSync = func(userId string, messageNumber int32) { + successfulSyncs = append(successfulSyncs, userId) + } + + // Add a user to sync + testUser := th.CreateUser() + _, _, appErr := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, testUser.Id, th.BasicUser.Id) + require.Nil(t, appErr) + _, appErr = th.App.AddUserToChannel(th.Context, testUser, channel, false) + require.Nil(t, appErr) + + // First sync attempt - will fail + err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) + require.NoError(t, err) + + // Wait for first sync attempt with more robust checking + require.Eventually(t, func() bool { + attempts := atomic.LoadInt32(&syncAttempts) + return attempts > 0 + }, 15*time.Second, 100*time.Millisecond, "Should have attempted sync during failure mode") + + initialAttempts := atomic.LoadInt32(&syncAttempts) + assert.Greater(t, initialAttempts, int32(0), "Should have attempted sync") + assert.Empty(t, successfulSyncs, "No successful syncs during failure mode") + + // Recover from failure mode + failureMode.Store(false) + + // Second sync attempt - should succeed + err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) + require.NoError(t, err) + + // Wait for successful sync with more robust checking + require.Eventually(t, func() bool { + for _, syncedUserId := range successfulSyncs { + if syncedUserId == testUser.Id { + return true + } + } + return false + }, 15*time.Second, 100*time.Millisecond, "Should have successful sync after recovery") + + // Verify recovery + finalAttempts := atomic.LoadInt32(&syncAttempts) + assert.Greater(t, finalAttempts, initialAttempts, "Should have retried after recovery") + }) + t.Run("Test 5: Manual sync with cursor management", func(t *testing.T) { + // This test verifies manual sync using SyncAllChannelMembers with complete cursor management: + // 1. Initial sync of 10 users with cursor tracking + // 2. Mixed operations: remove 3 users and add 5 new users + // 3. Verifies all operations are properly synced and cursor is updated correctly + // 4. Validates that the LastMembersSyncAt cursor advances after each sync operation + EnsureCleanState(t, th, ss) + var totalSyncMessages int32 + var addOperations int32 + var removeOperations int32 + var selfCluster *model.RemoteCluster + + // Create sync handler + syncHandler := &SelfReferentialSyncHandler{ + t: t, + service: service, + selfCluster: nil, // Will be set later + syncMessageCount: &totalSyncMessages, + } + + // Create test HTTP server + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/v4/remotecluster/msg" { + bodyBytes, _ := io.ReadAll(r.Body) + + // Count add and remove operations + var frame model.RemoteClusterFrame + if json.Unmarshal(bodyBytes, &frame) == nil { + var syncMsg model.SyncMsg + if json.Unmarshal(frame.Msg.Payload, &syncMsg) == nil && frame.Msg.Topic == "sharedchannel_membership" { + // Count membership changes from the unified field + for _, change := range syncMsg.MembershipChanges { + if change.IsAdd { + atomic.AddInt32(&addOperations, 1) + } else { + atomic.AddInt32(&removeOperations, 1) + } + } + } + } + + // Restore body and handle with sync handler + r.Body = io.NopCloser(bytes.NewReader(bodyBytes)) + if selfCluster != nil { + syncHandler.selfCluster = selfCluster + syncHandler.HandleRequest(w, r) + return + } + } + writeOKResponse(w) + })) + defer testServer.Close() + + // Create and share channel + channel := th.CreateChannel(th.Context, th.BasicTeam) + sc := &model.SharedChannel{ + ChannelId: channel.Id, + TeamId: th.BasicTeam.Id, + Home: true, + ShareName: "full-sync-test", + CreatorId: th.BasicUser.Id, + RemoteId: "", + } + _, shareErr := th.App.ShareChannel(th.Context, sc) + require.NoError(t, shareErr) + + // Create self-referential remote cluster + selfCluster = &model.RemoteCluster{ + RemoteId: model.NewId(), + Name: "self-cluster-full", + SiteURL: testServer.URL, + CreateAt: model.GetMillis(), + LastPingAt: model.GetMillis(), + Token: model.NewId(), + CreatorId: th.BasicUser.Id, + RemoteToken: model.NewId(), + } + selfCluster, err = ss.RemoteCluster().Save(selfCluster) + require.NoError(t, err) + + syncHandler.selfCluster = selfCluster + + scr := &model.SharedChannelRemote{ + Id: model.NewId(), + ChannelId: channel.Id, + CreatorId: th.BasicUser.Id, + RemoteId: selfCluster.RemoteId, + IsInviteAccepted: true, + IsInviteConfirmed: true, + LastMembersSyncAt: 0, + } + _, err = ss.SharedChannel().SaveRemote(scr) + require.NoError(t, err) + + // Refresh the channel object to get the updated Shared field + channel, appErr := th.App.GetChannel(th.Context, channel.Id) + require.Nil(t, appErr) + require.True(t, channel.IsShared(), "Channel should be marked as shared") + + // Phase 1: Add initial batch of users + initialUsers := make([]*model.User, 10) + for i := 0; i < 10; i++ { + initialUsers[i] = th.CreateUser() + _, _, appErr = th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, initialUsers[i].Id, th.BasicUser.Id) + require.Nil(t, appErr) + _, appErr = th.App.AddUserToChannel(th.Context, initialUsers[i], channel, false) + require.Nil(t, appErr) + } + + // Get initial cursor value + initialScr, scrErr := ss.SharedChannel().GetRemoteByIds(channel.Id, selfCluster.RemoteId) + require.NoError(t, scrErr) + initialCursor := initialScr.LastMembersSyncAt + + // Initial sync + err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) + require.NoError(t, err) + + // Wait for initial sync to complete + require.Eventually(t, func() bool { + return atomic.LoadInt32(&addOperations) >= 10 + }, 10*time.Second, 100*time.Millisecond, "Should sync all initial users") + + initialAdds := atomic.LoadInt32(&addOperations) + assert.GreaterOrEqual(t, initialAdds, int32(10), "Should sync all initial users") + + // Verify cursor was updated after initial sync + require.Eventually(t, func() bool { + updatedScr, getErr := ss.SharedChannel().GetRemoteByIds(channel.Id, selfCluster.RemoteId) + return getErr == nil && updatedScr.LastMembersSyncAt > initialCursor + }, 10*time.Second, 100*time.Millisecond, "Cursor should be updated after initial sync") + + // Get cursor after initial sync + afterInitialScr, getErr := ss.SharedChannel().GetRemoteByIds(channel.Id, selfCluster.RemoteId) + require.NoError(t, getErr) + cursorAfterInitial := afterInitialScr.LastMembersSyncAt + + // Phase 2: Mixed operations - remove some, add new ones + // Remove first 3 users + for i := 0; i < 3; i++ { + appErr := th.App.RemoveUserFromChannel(th.Context, initialUsers[i].Id, th.SystemAdminUser.Id, channel) + require.Nil(t, appErr) + } + + // Add 5 new users + newUsers := make([]*model.User, 5) + for i := 0; i < 5; i++ { + newUsers[i] = th.CreateUser() + _, _, appErr := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, newUsers[i].Id, th.BasicUser.Id) + require.Nil(t, appErr) + _, appErr = th.App.AddUserToChannel(th.Context, newUsers[i], channel, false) + require.Nil(t, appErr) + } + + // Sync mixed changes + previousMessages := atomic.LoadInt32(&totalSyncMessages) + + err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) + require.NoError(t, err) + + // Wait for mixed changes sync to complete + require.Eventually(t, func() bool { + messages := atomic.LoadInt32(&totalSyncMessages) + removes := atomic.LoadInt32(&removeOperations) + return messages > previousMessages && removes >= 3 + }, 10*time.Second, 100*time.Millisecond, "Should sync mixed changes") + + // Verify cursor was updated after mixed operations sync + require.Eventually(t, func() bool { + finalScr, finalErr := ss.SharedChannel().GetRemoteByIds(channel.Id, selfCluster.RemoteId) + return finalErr == nil && finalScr.LastMembersSyncAt > cursorAfterInitial + }, 10*time.Second, 100*time.Millisecond, "Cursor should be updated after mixed operations sync") + + // Verify final state + members, membersErr := ss.Channel().GetMembers(model.ChannelMembersGetOptions{ + ChannelID: channel.Id, + Limit: 100, + }) + require.NoError(t, membersErr) + + expectedMembers := 10 - 3 + 5 + 1 // initial - removed + added + system admin + assert.Equal(t, expectedMembers, len(members), "Should have correct final member count") + + finalMessages := atomic.LoadInt32(&totalSyncMessages) + finalAdds := atomic.LoadInt32(&addOperations) + finalRemoves := atomic.LoadInt32(&removeOperations) + + assert.Greater(t, finalMessages, int32(0), "Should have sync messages") + assert.Greater(t, finalAdds, int32(0), "Should have add operations") + assert.GreaterOrEqual(t, finalRemoves, int32(3), "Should have remove operations") + }) + t.Run("Test 6: Multiple remote clusters", func(t *testing.T) { + // This test verifies membership sync across multiple remote clusters: + // 1. Adding users syncs to all 3 remote clusters + // 2. Changes from one cluster propagate through our server to other clusters + // 3. Removals sync to all clusters + EnsureCleanState(t, th, ss) + var totalSyncMessages int32 + var syncMessagesPerCluster = make(map[string]*int32) + + // Create multiple test HTTP servers to simulate different remote clusters + clusters := make([]*model.RemoteCluster, 3) + testServers := make([]*httptest.Server, 3) + syncHandlers := make([]*SelfReferentialSyncHandler, 3) + + // Create 3 remote clusters and their servers + for i := 0; i < 3; i++ { + clusterName := fmt.Sprintf("cluster-%d", i+1) + var count int32 + syncMessagesPerCluster[clusterName] = &count + + // Create sync handler for this cluster + syncHandlers[i] = &SelfReferentialSyncHandler{ + t: t, + service: service, + selfCluster: nil, // Will be set later + syncMessageCount: &totalSyncMessages, + } + + // Create test server for this cluster + idx := i // Capture index for closure + testServers[i] = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/v4/remotecluster/msg" { + clusterName := fmt.Sprintf("cluster-%d", idx+1) + atomic.AddInt32(syncMessagesPerCluster[clusterName], 1) + + // Read body + bodyBytes, readErr := io.ReadAll(r.Body) + if readErr != nil { + writeOKResponse(w) + return + } + + r.Body = io.NopCloser(bytes.NewReader(bodyBytes)) + + // Handle with the appropriate sync handler + if clusters[idx] != nil { + syncHandlers[idx].selfCluster = clusters[idx] + syncHandlers[idx].HandleRequest(w, r) + return + } + } + writeOKResponse(w) + })) + } + + // Cleanup servers + defer func() { + for _, server := range testServers { + server.Close() + } + }() + + // Create a new team for this test to avoid team member limits + team := th.CreateTeam() + + // Create and share channel in the new team + channel := &model.Channel{ + TeamId: team.Id, + Name: "multi-cluster-test-channel", + DisplayName: "Multi Cluster Test Channel", + Type: model.ChannelTypeOpen, + } + var appErr *model.AppError + channel, appErr = th.App.CreateChannel(th.Context, channel, false) + require.Nil(t, appErr) + sc := &model.SharedChannel{ + ChannelId: channel.Id, + TeamId: team.Id, + Home: true, + ShareName: "multi-cluster-test", + CreatorId: th.BasicUser.Id, + RemoteId: "", // Empty means sync to all remotes + } + _, shareErr := th.App.ShareChannel(th.Context, sc) + require.NoError(t, shareErr) + + // Create remote clusters + for i := 0; i < 3; i++ { + clusters[i] = &model.RemoteCluster{ + RemoteId: model.NewId(), + Name: fmt.Sprintf("cluster-%d", i+1), + SiteURL: testServers[i].URL, + CreateAt: model.GetMillis(), + LastPingAt: model.GetMillis(), + Token: model.NewId(), + CreatorId: th.BasicUser.Id, + RemoteToken: model.NewId(), + } + clusters[i], err = ss.RemoteCluster().Save(clusters[i]) + require.NoError(t, err) + + // Share channel with this cluster + scr := &model.SharedChannelRemote{ + Id: model.NewId(), + ChannelId: channel.Id, + CreatorId: th.BasicUser.Id, + RemoteId: clusters[i].RemoteId, + IsInviteAccepted: true, + IsInviteConfirmed: true, + LastMembersSyncAt: 0, + } + _, err = ss.SharedChannel().SaveRemote(scr) + require.NoError(t, err) + } + + // Add users to channel - they should sync to all remote clusters + users := make([]*model.User, 5) + for i := 0; i < 5; i++ { + users[i] = th.CreateUser() + _, _, addErr := th.App.AddUserToTeam(th.Context, team.Id, users[i].Id, th.BasicUser.Id) + require.Nil(t, addErr) + _, addErr = th.App.AddUserToChannel(th.Context, users[i], channel, false) + require.Nil(t, addErr) + } + + // Sync to all clusters - need to sync each one individually + for _, cluster := range clusters { + err = service.SyncAllChannelMembers(channel.Id, cluster.RemoteId, nil) + require.NoError(t, err) + } + + // Wait for syncs to complete + require.Eventually(t, func() bool { + // Each cluster should receive at least 5 sync messages (one per user) + for _, countPtr := range syncMessagesPerCluster { + if atomic.LoadInt32(countPtr) < 5 { + return false + } + } + return true + }, 10*time.Second, 100*time.Millisecond, "All clusters should receive sync messages") + + // Verify each cluster received messages + for name, countPtr := range syncMessagesPerCluster { + finalCount := atomic.LoadInt32(countPtr) + assert.GreaterOrEqual(t, finalCount, int32(5), + "Cluster %s should receive at least 5 sync messages", name) + } + + // Part 2: Test propagation from one cluster through another + // This simulates cluster-2 receiving a membership change and propagating it + + // Reset counters + atomic.StoreInt32(&totalSyncMessages, 0) + for _, countPtr := range syncMessagesPerCluster { + atomic.StoreInt32(countPtr, 0) + } + + // Create a new user that will be added "by cluster-2" + userFromCluster2 := th.CreateUser() + _, _, appErr = th.App.AddUserToTeam(th.Context, team.Id, userFromCluster2.Id, th.BasicUser.Id) + require.Nil(t, appErr) + + // Create a sync message as if it came from cluster-2 adding this user + syncMsg := model.NewSyncMsg(channel.Id) + syncMsg.MembershipChanges = []*model.MembershipChangeMsg{ + { + ChannelId: channel.Id, + UserId: userFromCluster2.Id, + IsAdd: true, + RemoteId: clusters[1].RemoteId, // from cluster-2 + ChangeTime: model.GetMillis(), + }, + } + + // Wrap it in a RemoteClusterMsg + payload, payloadErr := syncMsg.ToJSON() + require.NoError(t, payloadErr) + + rcMsg := model.RemoteClusterMsg{ + Topic: sharedchannel.TopicSync, + Payload: payload, + } + + // Simulate cluster-2 sending this change to our server + // This should trigger our server to propagate to other clusters + response := &remotecluster.Response{} + err = service.OnReceiveSyncMessageForTesting(rcMsg, clusters[1], response) + require.NoError(t, err) + + // Our server should now propagate this change to cluster-1 and cluster-3 + // Wait for the propagation to happen + require.Eventually(t, func() bool { + count3 := atomic.LoadInt32(syncMessagesPerCluster["cluster-3"]) + // We expect at least cluster-3 to receive the propagated change + return count3 >= 1 + }, 10*time.Second, 100*time.Millisecond, "Change should propagate to other clusters") + + // Verify the user was added locally + member, memberErr := ss.Channel().GetMember(context.Background(), channel.Id, userFromCluster2.Id) + require.NoError(t, memberErr, "User should be a member after receiving sync from cluster-2") + require.Equal(t, userFromCluster2.Id, member.UserId) + + // Verify cluster-3 received the propagated change + finalCount3 := atomic.LoadInt32(syncMessagesPerCluster["cluster-3"]) + assert.GreaterOrEqual(t, finalCount3, int32(1), + "cluster-3 should receive propagated sync from our server") + + // Part 3: Test removal syncing to all clusters + + // Reset counters + atomic.StoreInt32(&totalSyncMessages, 0) + for _, countPtr := range syncMessagesPerCluster { + atomic.StoreInt32(countPtr, 0) + } + + // Remove a user - should sync to all clusters + appErr = th.App.RemoveUserFromChannel(th.Context, users[0].Id, th.SystemAdminUser.Id, channel) + require.Nil(t, appErr) + + // Sync removal to all clusters + for _, cluster := range clusters { + err = service.SyncAllChannelMembers(channel.Id, cluster.RemoteId, nil) + require.NoError(t, err) + } + + // Wait for removal sync + require.Eventually(t, func() bool { + perCluster := make(map[string]int32) + for name, countPtr := range syncMessagesPerCluster { + perCluster[name] = atomic.LoadInt32(countPtr) + } + + // Each cluster should receive at least 1 sync message for the removal + allClustersReceived := true + for _, count := range perCluster { + if count < 1 { + allClustersReceived = false + break + } + } + return allClustersReceived + }, 10*time.Second, 100*time.Millisecond, "All clusters should receive removal sync") + }) + t.Run("Test 7: Feature flag disabled", func(t *testing.T) { + // This test verifies that the shared channel membership sync functionality respects the feature flag. + // It tests two scenarios: + // 1. When the feature flag is disabled, no sync messages should be sent even when SyncAllChannelMembers is called + // 2. When the feature flag is enabled, sync messages should be sent as expected + // This ensures that the feature can be safely disabled in production without triggering unintended syncs + EnsureCleanState(t, th, ss) + var syncMessageCount int32 + + // Disable feature flag from the beginning to prevent any automatic sync + os.Setenv("MM_FEATUREFLAGS_ENABLESHAREDCHANNELMEMBERSYNC", "false") + rErr := th.App.ReloadConfig() + require.NoError(t, rErr) + + // Create test HTTP server that counts sync messages + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/v4/remotecluster/msg" { + atomic.AddInt32(&syncMessageCount, 1) + } + writeOKResponse(w) + })) + defer testServer.Close() + + // Create and share channel + channel := th.CreateChannel(th.Context, th.BasicTeam) + sc := &model.SharedChannel{ + ChannelId: channel.Id, + TeamId: th.BasicTeam.Id, + Home: true, + ShareName: "feature-flag-test", + CreatorId: th.BasicUser.Id, + RemoteId: "", + } + _, shareErr := th.App.ShareChannel(th.Context, sc) + require.NoError(t, shareErr) + + // Create self-referential remote cluster + selfCluster := &model.RemoteCluster{ + RemoteId: model.NewId(), + Name: "self-cluster-flag-test", + SiteURL: testServer.URL, + CreateAt: model.GetMillis(), + LastPingAt: model.GetMillis(), + Token: model.NewId(), + CreatorId: th.BasicUser.Id, + RemoteToken: model.NewId(), + } + selfCluster, err = ss.RemoteCluster().Save(selfCluster) + require.NoError(t, err) + + // Share channel with self + scr := &model.SharedChannelRemote{ + Id: model.NewId(), + ChannelId: channel.Id, + CreatorId: th.BasicUser.Id, + RemoteId: selfCluster.RemoteId, + IsInviteAccepted: true, + IsInviteConfirmed: true, + LastMembersSyncAt: 0, + } + _, err = ss.SharedChannel().SaveRemote(scr) + require.NoError(t, err) + + // Test 1: Sync with feature flag disabled + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.FeatureFlags.EnableSharedChannelsMemberSync = false + }) + + // Add users to the channel after disabling the feature flag + for i := 0; i < 3; i++ { + user := th.CreateUser() + _, _, appErr := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, user.Id, th.BasicUser.Id) + require.Nil(t, appErr) + _, appErr = th.App.AddUserToChannel(th.Context, user, channel, false) + require.Nil(t, appErr) + } + + atomic.StoreInt32(&syncMessageCount, 0) + err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) + require.NoError(t, err) + + // Verify no sync messages were sent + require.Never(t, func() bool { + return atomic.LoadInt32(&syncMessageCount) > 0 + }, 2*time.Second, 100*time.Millisecond, "No sync should occur with feature flag disabled") + + // Test 2: Sync with feature flag enabled + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.FeatureFlags.EnableSharedChannelsMemberSync = true + }) + + atomic.StoreInt32(&syncMessageCount, 0) + err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) + require.NoError(t, err) + + // Verify sync messages were sent + require.Eventually(t, func() bool { + return atomic.LoadInt32(&syncMessageCount) > 0 + }, 5*time.Second, 100*time.Millisecond, "Sync should occur with feature flag enabled") + }) + t.Run("Test 8: Sync Task After Connection Becomes Available", func(t *testing.T) { + // This test verifies that membership sync works correctly when a remote cluster + // reconnects after being offline. We simulate a cluster that has been unavailable + // (old LastPingAt timestamp) and then comes back online. + // Expected behavior: + // 1. Cluster is created with old LastPingAt (simulating previous offline state) + // 2. When LastPingAt is updated (cluster comes online), sync should work + // 3. All channel members should be synced successfully + // 4. Cursor (LastMembersSyncAt) should be updated after successful sync + EnsureCleanState(t, th, ss) + + var syncTaskCreated atomic.Bool + var syncHandler *SelfReferentialSyncHandler + + // Create test HTTP server + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if syncHandler != nil { + syncHandler.HandleRequest(w, r) + } else { + writeOKResponse(w) + } + })) + defer testServer.Close() + + // Create channel and share it + channel := th.CreateChannel(th.Context, th.BasicTeam) + sc := &model.SharedChannel{ + ChannelId: channel.Id, + TeamId: th.BasicTeam.Id, + Home: true, + ShareName: "reconnect-test", + CreatorId: th.BasicUser.Id, + RemoteId: "", + } + _, shareErr := th.App.ShareChannel(th.Context, sc) + require.NoError(t, shareErr) + + // Create remote cluster that was previously offline (old LastPingAt) + selfCluster := &model.RemoteCluster{ + RemoteId: model.NewId(), + Name: "self-cluster-reconnect", + SiteURL: testServer.URL, + CreateAt: model.GetMillis() - 300000, // Created 5 minutes ago + LastPingAt: model.GetMillis() - 120000, // Last ping 2 minutes ago + Token: model.NewId(), + CreatorId: th.BasicUser.Id, + RemoteToken: model.NewId(), + } + selfCluster, err = ss.RemoteCluster().Save(selfCluster) + require.NoError(t, err) + + // Share channel with self + scr := &model.SharedChannelRemote{ + Id: model.NewId(), + ChannelId: channel.Id, + CreatorId: th.BasicUser.Id, + RemoteId: selfCluster.RemoteId, + IsInviteAccepted: true, + IsInviteConfirmed: true, + LastMembersSyncAt: 0, + } + _, err = ss.SharedChannel().SaveRemote(scr) + require.NoError(t, err) + + // Initialize sync handler + syncHandler = NewSelfReferentialSyncHandler(t, service, selfCluster) + syncHandler.OnBatchSync = func(userIds []string, messageNumber int32) { + syncTaskCreated.Store(true) + } + + // Add some users to sync + for i := 0; i < 3; i++ { + user := th.CreateUser() + _, _, appErr := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, user.Id, th.BasicUser.Id) + require.Nil(t, appErr) + _, appErr = th.App.AddUserToChannel(th.Context, user, channel, false) + require.Nil(t, appErr) + } + + // Update LastPingAt to simulate cluster coming back online + selfCluster.LastPingAt = model.GetMillis() + _, err = ss.RemoteCluster().Update(selfCluster) + require.NoError(t, err) + + // Trigger membership sync as would happen when connection is restored + err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) + require.NoError(t, err) + + // Verify sync task was created and executed with more generous timeout + require.Eventually(t, func() bool { + return syncTaskCreated.Load() + }, 15*time.Second, 200*time.Millisecond, "Sync should execute when cluster comes back online") + + // Wait for async task queue to be processed + require.Eventually(t, func() bool { + return !service.HasPendingTasksForTesting() + }, 10*time.Second, 200*time.Millisecond, "All async sync tasks should be completed") + + // Verify cursor was updated with extended timeout + require.Eventually(t, func() bool { + updatedScr, scrErr := ss.SharedChannel().GetRemoteByIds(channel.Id, selfCluster.RemoteId) + return scrErr == nil && updatedScr.LastMembersSyncAt > 0 + }, 20*time.Second, 200*time.Millisecond, "Cursor should be updated after sync") + }) + t.Run("Test 9: Remote Cluster Offline During Sync", func(t *testing.T) { + // This test verifies graceful failure handling when a remote cluster goes offline + // during the sync operation. We simulate a server that works initially but then + // becomes unavailable during subsequent sync attempts. + // Expected behavior: + // 1. First sync succeeds and updates the cursor + // 2. Server goes offline for the second sync attempt + // 3. Second sync fails gracefully without errors + // 4. Cursor remains at the value from the successful first sync (no corruption) + // 5. No partial data is persisted from the failed sync + EnsureCleanState(t, th, ss) + + var syncAttempts int32 + var serverOnline atomic.Bool + serverOnline.Store(true) + var syncHandler *SelfReferentialSyncHandler + + // Create test HTTP server that can simulate going offline + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !serverOnline.Load() { + // Simulate server being offline + w.WriteHeader(http.StatusServiceUnavailable) + return + } + + if r.URL.Path == "/api/v4/remotecluster/msg" { + currentAttempt := atomic.AddInt32(&syncAttempts, 1) + // On second sync cycle, go offline (allow first full sync to complete) + if currentAttempt > 2 { + serverOnline.Store(false) + w.WriteHeader(http.StatusServiceUnavailable) + return + } + + // First sync succeeds - use sync handler if available for all requests in first sync + if syncHandler != nil { + syncHandler.HandleRequest(w, r) + return + } + } + writeOKResponse(w) + })) + defer testServer.Close() + + // Create channel and share it + channel := th.CreateChannel(th.Context, th.BasicTeam) + sc := &model.SharedChannel{ + ChannelId: channel.Id, + TeamId: th.BasicTeam.Id, + Home: true, + ShareName: "offline-test", + CreatorId: th.BasicUser.Id, + RemoteId: "", + } + _, shareErr := th.App.ShareChannel(th.Context, sc) + require.NoError(t, shareErr) + + // Create remote cluster + selfCluster := &model.RemoteCluster{ + RemoteId: model.NewId(), + Name: "self-cluster-offline", + SiteURL: testServer.URL, + CreateAt: model.GetMillis(), + LastPingAt: model.GetMillis(), + Token: model.NewId(), + CreatorId: th.BasicUser.Id, + RemoteToken: model.NewId(), + } + selfCluster, err = ss.RemoteCluster().Save(selfCluster) + require.NoError(t, err) + + // Share channel with self + scr := &model.SharedChannelRemote{ + Id: model.NewId(), + ChannelId: channel.Id, + CreatorId: th.BasicUser.Id, + RemoteId: selfCluster.RemoteId, + IsInviteAccepted: true, + IsInviteConfirmed: true, + LastMembersSyncAt: 0, + } + _, err = ss.SharedChannel().SaveRemote(scr) + require.NoError(t, err) + + // Add users to sync - use more than batch size to test batch sync + // Default batch size is 20, so use 25 users to ensure batch processing + for i := 0; i < 25; i++ { + user := th.CreateUser() + _, _, appErr := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, user.Id, th.BasicUser.Id) + require.Nil(t, appErr) + _, appErr = th.App.AddUserToChannel(th.Context, user, channel, false) + require.Nil(t, appErr) + } + + // Initialize sync handler + syncHandler = NewSelfReferentialSyncHandler(t, service, selfCluster) + + // First sync should succeed + err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) + require.NoError(t, err) + + // Wait for first sync with more generous timeout + require.Eventually(t, func() bool { + return atomic.LoadInt32(&syncAttempts) >= 1 + }, 15*time.Second, 200*time.Millisecond, "Should complete first sync") + + // Wait for cursor to be updated after first sync + var firstCursor int64 + require.Eventually(t, func() bool { + scr1, scrErr := ss.SharedChannel().GetRemoteByIds(channel.Id, selfCluster.RemoteId) + if scrErr != nil { + return false + } + firstCursor = scr1.LastMembersSyncAt + return firstCursor > 0 + }, 10*time.Second, 200*time.Millisecond, "Cursor should be set after first sync") + + // Add more users to ensure we still have > 20 total for batch sync + for i := 0; i < 5; i++ { + user := th.CreateUser() + _, _, appErr := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, user.Id, th.BasicUser.Id) + require.Nil(t, appErr) + _, appErr = th.App.AddUserToChannel(th.Context, user, channel, false) + require.Nil(t, appErr) + } + + // Second sync should fail (server goes offline) + err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) + require.NoError(t, err) // Method itself shouldn't error + + // Wait for second sync attempt with more generous timeout + require.Eventually(t, func() bool { + return atomic.LoadInt32(&syncAttempts) >= 2 + }, 20*time.Second, 200*time.Millisecond, "Should attempt second sync") + + // Wait for any cursor updates to complete and verify cursor was not updated + require.Never(t, func() bool { + scr2, scrErr := ss.SharedChannel().GetRemoteByIds(channel.Id, selfCluster.RemoteId) + if scrErr != nil { + return false + } + return scr2.LastMembersSyncAt > firstCursor + }, 5*time.Second, 200*time.Millisecond, "Cursor should not update when sync fails") + }) + t.Run("Test 10: Users in Multiple Shared Channels", func(t *testing.T) { + // This test verifies deduplication when users belong to multiple shared channels. + // Unlike global user sync which syncs users once regardless of channel membership, + // membership sync operates per channel and must handle users in multiple channels. + // Test setup: + // - user1: member of all 3 shared channels + // - user2: member of 2 shared channels (channel1 and channel2) + // - user3: member of 1 shared channel (channel3) + // Expected behavior: + // - Each user is synced exactly once per channel they belong to + // - No duplicate sync messages for the same user in the same channel + // - Users not in a channel are not synced for that channel + EnsureCleanState(t, th, ss) + + var syncedChannelUsers = make(map[string][]string) // channelId -> userIds + var mu sync.Mutex + var syncHandler *SelfReferentialSyncHandler + var testServer *httptest.Server + var totalSyncMessages int32 + + // Create users + user1 := th.CreateUser() + user2 := th.CreateUser() + user3 := th.CreateUser() + + // Add users to team + th.LinkUserToTeam(user1, th.BasicTeam) + th.LinkUserToTeam(user2, th.BasicTeam) + th.LinkUserToTeam(user3, th.BasicTeam) + + // Create multiple shared channels + channel1 := th.CreateChannel(th.Context, th.BasicTeam) + channel2 := th.CreateChannel(th.Context, th.BasicTeam) + channel3 := th.CreateChannel(th.Context, th.BasicTeam) + + // Add users to multiple shared channels + // user1 in all channels + th.AddUserToChannel(user1, channel1) + th.AddUserToChannel(user1, channel2) + th.AddUserToChannel(user1, channel3) + + // user2 in two channels + th.AddUserToChannel(user2, channel1) + th.AddUserToChannel(user2, channel2) + + // user3 in one channel + th.AddUserToChannel(user3, channel3) + + // First create the remote cluster with a placeholder URL + selfCluster := &model.RemoteCluster{ + RemoteId: model.NewId(), + Name: "self-cluster-multi-channel", + SiteURL: "http://placeholder", + CreateAt: model.GetMillis(), + LastPingAt: model.GetMillis(), + Token: model.NewId(), + CreatorId: th.BasicUser.Id, + RemoteToken: model.NewId(), + } + selfCluster, err = ss.RemoteCluster().Save(selfCluster) + require.NoError(t, err) + + // Initialize sync handler before creating the test server + syncHandler = NewSelfReferentialSyncHandler(t, service, selfCluster) + + // Create a wrapper handler to intercept sync messages + testServer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Intercept and track channel syncs + if r.URL.Path == "/api/v4/remotecluster/msg" { + bodyBytes, pErr := io.ReadAll(r.Body) + if pErr == nil { + // Restore body for actual handler + r.Body = io.NopCloser(bytes.NewReader(bodyBytes)) + + var frame model.RemoteClusterFrame + if json.Unmarshal(bodyBytes, &frame) == nil { + var syncMsg model.SyncMsg + if json.Unmarshal(frame.Msg.Payload, &syncMsg) == nil { + mu.Lock() + channelId := syncMsg.ChannelId + if channelId != "" { + if _, ok := syncedChannelUsers[channelId]; !ok { + syncedChannelUsers[channelId] = []string{} + } + + // Add all users from membership changes + for _, change := range syncMsg.MembershipChanges { + if change.IsAdd { + syncedChannelUsers[channelId] = append(syncedChannelUsers[channelId], change.UserId) + atomic.AddInt32(&totalSyncMessages, 1) + } + } + } + mu.Unlock() + } + } + } + } + + // Call the actual sync handler + syncHandler.HandleRequest(w, r) + })) + defer testServer.Close() + + // Update the cluster with the actual test server URL + selfCluster.SiteURL = testServer.URL + _, err = ss.RemoteCluster().Update(selfCluster) + require.NoError(t, err) + + // Make channels shared + for i, channel := range []*model.Channel{channel1, channel2, channel3} { + sc := &model.SharedChannel{ + ChannelId: channel.Id, + TeamId: th.BasicTeam.Id, + RemoteId: selfCluster.RemoteId, + Home: true, + ReadOnly: false, + ShareName: fmt.Sprintf("channel%d", i+1), + CreatorId: th.BasicUser.Id, + } + _, err = ss.SharedChannel().Save(sc) + require.NoError(t, err) + + scr := &model.SharedChannelRemote{ + Id: model.NewId(), + ChannelId: channel.Id, + CreatorId: th.BasicUser.Id, + RemoteId: selfCluster.RemoteId, + IsInviteAccepted: true, + IsInviteConfirmed: true, + LastMembersSyncAt: 0, + } + _, err = ss.SharedChannel().SaveRemote(scr) + require.NoError(t, err) + } + + // Sync memberships for each channel separately + for _, channel := range []*model.Channel{channel1, channel2, channel3} { + err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) + require.NoError(t, err) + } + + // Ensure the sync handler is ready by waiting for the first message + require.Eventually(t, func() bool { + return atomic.LoadInt32(&totalSyncMessages) > 0 + }, 10*time.Second, 50*time.Millisecond, "Expected at least one sync message to be sent") + + // Calculate expected number of sync messages + // channel1: user1, user2, BasicUser = 3 + // channel2: user1, user2, BasicUser = 3 + // channel3: user1, user3, BasicUser = 3 + // Total: 9 sync messages (one per user per channel) + expectedSyncMessages := int32(9) + + // Wait for all sync messages to be processed with detailed debugging + require.Eventually(t, func() bool { + currentMessages := atomic.LoadInt32(&totalSyncMessages) + + mu.Lock() + channelCount := len(syncedChannelUsers) + var totalUsers int + for _, users := range syncedChannelUsers { + totalUsers += len(users) + } + mu.Unlock() + + // Log progress for debugging + if currentMessages < expectedSyncMessages { + t.Logf("Waiting for sync messages: %d/%d received, %d channels synced, %d total users", + currentMessages, expectedSyncMessages, channelCount, totalUsers) + } + + return currentMessages >= expectedSyncMessages + }, 30*time.Second, 200*time.Millisecond, + fmt.Sprintf("Expected %d sync messages, but got %d", expectedSyncMessages, atomic.LoadInt32(&totalSyncMessages))) + + // Verify we have complete data for all channels + require.Eventually(t, func() bool { + mu.Lock() + defer mu.Unlock() + + // Verify we have sync data for all 3 channels + if len(syncedChannelUsers) != 3 { + return false + } + + // Verify each channel has the expected number of users + channel1Users, ok1 := syncedChannelUsers[channel1.Id] + channel2Users, ok2 := syncedChannelUsers[channel2.Id] + channel3Users, ok3 := syncedChannelUsers[channel3.Id] + + if !ok1 || !ok2 || !ok3 { + return false + } + + // Count unique users per channel + channel1Count := len(getUniqueUsers(channel1Users)) + channel2Count := len(getUniqueUsers(channel2Users)) + channel3Count := len(getUniqueUsers(channel3Users)) + + // channel1 should have user1, user2, BasicUser = 3 + // channel2 should have user1, user2, BasicUser = 3 + // channel3 should have user1, user3, BasicUser = 3 + expectedCounts := channel1Count == 3 && channel2Count == 3 && channel3Count == 3 + + if !expectedCounts { + t.Logf("Channel user counts - channel1: %d, channel2: %d, channel3: %d", + channel1Count, channel2Count, channel3Count) + } + + return expectedCounts + }, 30*time.Second, 200*time.Millisecond, "Expected all channels to have their users synced") + + // Verify each user is synced exactly once per channel they belong to + mu.Lock() + defer mu.Unlock() + + // Verify channel1 synced user1, user2, and BasicUser + channel1Users := syncedChannelUsers[channel1.Id] + userCount1 := make(map[string]int) + for _, userId := range channel1Users { + userCount1[userId]++ + } + assert.Equal(t, 1, userCount1[user1.Id], "User1 should be synced exactly once for channel1") + assert.Equal(t, 1, userCount1[user2.Id], "User2 should be synced exactly once for channel1") + assert.Equal(t, 0, userCount1[user3.Id], "User3 should not be synced for channel1") + + // Verify channel2 synced user1, user2, and BasicUser + channel2Users := syncedChannelUsers[channel2.Id] + userCount2 := make(map[string]int) + for _, userId := range channel2Users { + userCount2[userId]++ + } + assert.Equal(t, 1, userCount2[user1.Id], "User1 should be synced exactly once for channel2") + assert.Equal(t, 1, userCount2[user2.Id], "User2 should be synced exactly once for channel2") + assert.Equal(t, 0, userCount2[user3.Id], "User3 should not be synced for channel2") + + // Verify channel3 synced user1, user3, and BasicUser + channel3Users := syncedChannelUsers[channel3.Id] + userCount3 := make(map[string]int) + for _, userId := range channel3Users { + userCount3[userId]++ + } + assert.Equal(t, 1, userCount3[user1.Id], "User1 should be synced exactly once for channel3") + assert.Equal(t, 0, userCount3[user2.Id], "User2 should not be synced for channel3") + assert.Equal(t, 1, userCount3[user3.Id], "User3 should be synced exactly once for channel3") + }) + // t.Run("Test 11: Strengthened conflict resolution", func(t *testing.T) { + // // This test verifies robust handling of conflicting membership states between multiple clusters. + // // Strengthened conflict scenarios tested: + // // 1. Basic conflict: user removed from one cluster but not another + // // 2. Re-addition after partial sync: user re-added after being synced to only one cluster + // // 3. Cursor consistency: verify sync cursors are properly updated across conflict resolution + // // 4. Post-conflict operations: ensure system works correctly after resolving conflicts + // // The test ensures the system correctly resolves conflicts and maintains data consistency + // EnsureCleanState(t, th, ss) + // var syncMessages []model.SyncMsg + // var mu sync.Mutex + // var syncMessageCount int32 + // var selfCluster *model.RemoteCluster + + // // Create sync handler + // syncHandler := NewSelfReferentialSyncHandler(t, service, nil) + + // // Create test HTTP server that tracks sync messages + // testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // if r.URL.Path == "/api/v4/remotecluster/msg" { + // atomic.AddInt32(&syncMessageCount, 1) + + // // Read body once + // bodyBytes, readErr := io.ReadAll(r.Body) + // if readErr != nil { + // writeOKResponse(w) + // return + // } + // r.Body = io.NopCloser(bytes.NewReader(bodyBytes)) + + // // Capture all sync messages - parse as RemoteClusterFrame + // var frame model.RemoteClusterFrame + // if unmarshalErr := json.Unmarshal(bodyBytes, &frame); unmarshalErr == nil { + // var syncMsg model.SyncMsg + // if payloadErr := json.Unmarshal(frame.Msg.Payload, &syncMsg); payloadErr == nil { + // mu.Lock() + // syncMessages = append(syncMessages, syncMsg) + // mu.Unlock() + // } + // } + + // // Handle with sync handler for proper response + // if selfCluster != nil { + // syncHandler.selfCluster = selfCluster + // syncHandler.HandleRequest(w, r) + // return + // } + // } + // writeOKResponse(w) + // })) + // defer testServer.Close() + + // // Create and share channel + // channel := th.CreateChannel(th.Context, th.BasicTeam) + // sc := &model.SharedChannel{ + // ChannelId: channel.Id, + // TeamId: th.BasicTeam.Id, + // Home: true, + // ShareName: "strengthened-conflict-test", + // CreatorId: th.BasicUser.Id, + // RemoteId: "", + // } + // _, shareErr := th.App.ShareChannel(th.Context, sc) + // require.NoError(t, shareErr) + + // // Create self-referential remote cluster + // selfCluster = &model.RemoteCluster{ + // RemoteId: model.NewId(), + // Name: "self-cluster-strengthened-conflict", + // SiteURL: testServer.URL, + // CreateAt: model.GetMillis(), + // LastPingAt: model.GetMillis(), + // Token: model.NewId(), + // CreatorId: th.BasicUser.Id, + // RemoteToken: model.NewId(), + // } + // selfCluster, err = ss.RemoteCluster().Save(selfCluster) + // require.NoError(t, err) + + // // Share channel with self + // scr := &model.SharedChannelRemote{ + // Id: model.NewId(), + // ChannelId: channel.Id, + // CreatorId: th.BasicUser.Id, + // RemoteId: selfCluster.RemoteId, + // IsInviteAccepted: true, + // IsInviteConfirmed: true, + // LastMembersSyncAt: 0, + // } + // _, err = ss.SharedChannel().SaveRemote(scr) + // require.NoError(t, err) + + // // Phase 1: Create users for conflict scenarios + // conflictUser1 := th.CreateUser() + // conflictUser2 := th.CreateUser() + + // // Add users to team + // for _, user := range []*model.User{conflictUser1, conflictUser2} { + // _, _, appErr := th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, user.Id, th.BasicUser.Id) + // require.Nil(t, appErr) + // } + + // // Add users to channel initially + // for _, user := range []*model.User{conflictUser1, conflictUser2} { + // _, appErr := th.App.AddUserToChannel(th.Context, user, channel, false) + // require.Nil(t, appErr) + // } + + // // Verify users are initially members + // for _, user := range []*model.User{conflictUser1, conflictUser2} { + // _, initialErr := ss.Channel().GetMember(context.Background(), channel.Id, user.Id) + // require.NoError(t, initialErr, "User should be initially a member") + // } + + // // Phase 2: Initial sync + // err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) + // require.NoError(t, err) + + // // Wait for initial sync to complete + // require.Eventually(t, func() bool { + // count := atomic.LoadInt32(&syncMessageCount) + // return count > 0 + // }, 15*time.Second, 200*time.Millisecond, "Should have initial sync messages") + + // // Get initial cursor + // initialScr, scrErr := ss.SharedChannel().GetRemoteByIds(channel.Id, selfCluster.RemoteId) + // require.NoError(t, scrErr) + // initialCursor := initialScr.LastMembersSyncAt + + // // Phase 3: Create conflict by removing user locally + // // Temporarily disable automatic sync to prevent interference with manual operations + // th.App.UpdateConfig(func(cfg *model.Config) { + // cfg.FeatureFlags.EnableSharedChannelsMemberSync = false + // }) + // rErr := th.App.ReloadConfig() + // require.NoError(t, rErr) + + // // Remove conflictUser1 locally + // appErr := th.App.RemoveUserFromChannel(th.Context, conflictUser1.Id, th.SystemAdminUser.Id, channel) + // require.Nil(t, appErr, "Failed to remove conflictUser1") + + // // Wait for removal to complete + // require.Eventually(t, func() bool { + // _, err1 := ss.Channel().GetMember(context.Background(), channel.Id, conflictUser1.Id) + // return err1 != nil // User should not be found + // }, 15*time.Second, 200*time.Millisecond, "User should be removed from channel") + + // // Phase 4: Re-add user (creating conflict state) + // // Re-add conflictUser1 (simulating conflict between local state and remote state) + // _, appErr = th.App.AddUserToChannel(th.Context, conflictUser1, channel, false) + // require.Nil(t, appErr) + + // // Re-enable automatic sync for the conflict resolution phase + // th.App.UpdateConfig(func(cfg *model.Config) { + // cfg.FeatureFlags.EnableSharedChannelsMemberSync = true + // }) + // rErr = th.App.ReloadConfig() + // require.NoError(t, rErr) + + // // Wait for local operation to complete + // require.Eventually(t, func() bool { + // _, err1 := ss.Channel().GetMember(context.Background(), channel.Id, conflictUser1.Id) + // return err1 == nil // User should be found + // }, 10*time.Second, 200*time.Millisecond, "User should be locally a member after re-adding") + + // // Phase 5: Conflict resolution sync + // // Reset message tracking for conflict resolution phase + // atomic.StoreInt32(&syncMessageCount, 0) + // mu.Lock() + // syncMessages = []model.SyncMsg{} + // mu.Unlock() + + // err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) + // require.NoError(t, err) + + // // Wait for conflict resolution sync to complete + // require.Eventually(t, func() bool { + // count := atomic.LoadInt32(&syncMessageCount) + // return count > 0 + // }, 20*time.Second, 200*time.Millisecond, "Should receive conflict resolution sync messages") + + // // Phase 6: Verification of conflict resolution + // mu.Lock() + // totalMessages := len(syncMessages) + // mu.Unlock() + + // // Verify we received sync messages for conflict resolution + // assert.Greater(t, totalMessages, 0, "Should have sync messages for conflict resolution") + + // // Verify final membership state is consistent + // expectedMembers := map[string]bool{ + // conflictUser1.Id: true, // Re-added, should be member + // conflictUser2.Id: true, // Never removed, should be member + // th.BasicUser.Id: true, // Basic user, should be member + // } + + // for userId, shouldBeMember := range expectedMembers { + // require.Eventually(t, func() bool { + // _, memberErr := ss.Channel().GetMember(context.Background(), channel.Id, userId) + // if shouldBeMember { + // return memberErr == nil + // } + // return memberErr != nil + // }, 10*time.Second, 200*time.Millisecond, + // "User %s membership state should be consistent after conflict resolution", userId) + // } + + // // Phase 7: Verify cursor updates + // require.Eventually(t, func() bool { + // scrUpdated, err1 := ss.SharedChannel().GetRemoteByIds(channel.Id, selfCluster.RemoteId) + // return err1 == nil && scrUpdated.LastMembersSyncAt > initialCursor + // }, 15*time.Second, 200*time.Millisecond, "Cluster should have updated sync cursor") + + // // Phase 8: Test that new operations after conflict resolution work correctly + // newUser := th.CreateUser() + // _, _, appErr = th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, newUser.Id, th.BasicUser.Id) + // require.Nil(t, appErr) + // _, appErr = th.App.AddUserToChannel(th.Context, newUser, channel, false) + // require.Nil(t, appErr) + + // // Reset and sync this new user + // atomic.StoreInt32(&syncMessageCount, 0) + // mu.Lock() + // syncMessages = []model.SyncMsg{} + // mu.Unlock() + + // err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) + // require.NoError(t, err) + + // // Verify the new user is synced correctly + // require.Eventually(t, func() bool { + // mu.Lock() + // defer mu.Unlock() + + // // Check if the new user appears in any sync message + // for _, msg := range syncMessages { + // for _, change := range msg.MembershipChanges { + // if change.UserId == newUser.Id && change.IsAdd { + // return true + // } + // } + // } + // return false + // }, 15*time.Second, 200*time.Millisecond, "New user should be synced correctly after conflict resolution") + + // // Phase 9: Verify efficiency - no redundant syncs for existing members + // atomic.StoreInt32(&syncMessageCount, 0) + // mu.Lock() + // syncMessages = []model.SyncMsg{} + // mu.Unlock() + + // // Trigger another sync - should not send messages for already-synced members + // err = service.SyncAllChannelMembers(channel.Id, selfCluster.RemoteId, nil) + // require.NoError(t, err) + + // // Wait for sync completion and verify minimal activity + // // Give time for any sync to complete, then check the final count + // require.Eventually(t, func() bool { + // finalCount := atomic.LoadInt32(&syncMessageCount) + // // Should have minimal activity since all members are already synced + // return finalCount <= 1 + // }, 10*time.Second, 200*time.Millisecond, "Should have minimal sync activity for already-synced members") + // }) +} + +// Helper function to get unique users from a list +func getUniqueUsers(users []string) map[string]bool { + unique := make(map[string]bool) + for _, user := range users { + unique[user] = true + } + return unique +} diff --git a/server/channels/app/shared_channel_service_iface.go b/server/channels/app/shared_channel_service_iface.go index 2ab50c14a58..7dc117d66c9 100644 --- a/server/channels/app/shared_channel_service_iface.go +++ b/server/channels/app/shared_channel_service_iface.go @@ -26,6 +26,7 @@ type SharedChannelServiceIFace interface { CheckChannelNotShared(channelID string) error CheckChannelIsShared(channelID string) error CheckCanInviteToSharedChannel(channelId string) error + HandleMembershipChange(channelID, userID string, isAdd bool, remoteID string) } func NewMockSharedChannelService(service SharedChannelServiceIFace) *mockSharedChannelService { @@ -91,3 +92,9 @@ func (mrcs *mockSharedChannelService) SendChannelInvite(channel *model.Channel, func (mrcs *mockSharedChannelService) NumInvitations() int { return mrcs.numInvitations } + +func (mrcs *mockSharedChannelService) HandleMembershipChange(channelID, userID string, isAdd bool, remoteID string) { + if mrcs.SharedChannelServiceIFace != nil { + mrcs.SharedChannelServiceIFace.HandleMembershipChange(channelID, userID, isAdd, remoteID) + } +} diff --git a/server/channels/app/shared_channel_sync_self_referential_utils_test.go b/server/channels/app/shared_channel_sync_self_referential_utils_test.go index 411ba70a118..b310246bc0e 100644 --- a/server/channels/app/shared_channel_sync_self_referential_utils_test.go +++ b/server/channels/app/shared_channel_sync_self_referential_utils_test.go @@ -105,6 +105,27 @@ func (h *SelfReferentialSyncHandler) HandleRequest(w http.ResponseWriter, r *htt } } + // Handle membership sync using unified field + if len(syncMsg.MembershipChanges) > 0 { + batch := make([]string, 0) + for _, change := range syncMsg.MembershipChanges { + if change.IsAdd { + syncResp.UsersSyncd = append(syncResp.UsersSyncd, change.UserId) + batch = append(batch, change.UserId) + } + } + + // Call appropriate callback + if len(batch) > 0 { + if h.OnBatchSync != nil { + h.OnBatchSync(batch, currentCall) + } + if len(batch) == 1 && h.OnIndividualSync != nil { + h.OnIndividualSync(batch[0], currentCall) + } + } + } + _ = response.SetPayload(syncResp) } } @@ -135,28 +156,36 @@ func (h *SelfReferentialSyncHandler) GetSyncMessageCount() int32 { return atomic.LoadInt32(h.syncMessageCount) } -// GetUsersFromSyncMsg extracts user IDs from a sync message -func GetUsersFromSyncMsg(msg model.SyncMsg) []string { - var userIds []string - - // Extract from users field - for userId := range msg.Users { - userIds = append(userIds, userId) - } - - return userIds -} - // EnsureCleanState ensures a clean test state by removing all shared channels, remote clusters, // and extra team/channel members. This helps prevent state pollution between tests. func EnsureCleanState(t *testing.T, th *TestHelper, ss store.Store) { t.Helper() + // First, wait for any pending async tasks to complete, then shutdown services + scsInterface := th.App.Srv().GetSharedChannelSyncService() + if scsInterface != nil && scsInterface.Active() { + // Cast to concrete type to access testing methods + if service, ok := scsInterface.(*sharedchannel.Service); ok { + // Wait for any pending tasks from previous tests to complete + require.Eventually(t, func() bool { + return !service.HasPendingTasksForTesting() + }, 10*time.Second, 100*time.Millisecond, "All pending sync tasks should complete before cleanup") + } + + // Shutdown the shared channel service to stop any async operations + _ = scsInterface.Shutdown() + + // Wait for shutdown to complete with more time + require.Eventually(t, func() bool { + return !scsInterface.Active() + }, 5*time.Second, 100*time.Millisecond, "Shared channel service should be inactive after shutdown") + } + // Clear all shared channels and remotes from previous tests allSharedChannels, _ := ss.SharedChannel().GetAll(0, 1000, model.SharedChannelFilterOpts{}) for _, sc := range allSharedChannels { // Delete all remotes for this channel - remotes, _ := ss.SharedChannel().GetRemotes(0, 100, model.SharedChannelRemoteFilterOpts{ChannelId: sc.ChannelId}) + remotes, _ := ss.SharedChannel().GetRemotes(0, 999999, model.SharedChannelRemoteFilterOpts{ChannelId: sc.ChannelId}) for _, remote := range remotes { _, _ = ss.SharedChannel().DeleteRemote(remote.Id) } @@ -170,13 +199,32 @@ func EnsureCleanState(t *testing.T, th *TestHelper, ss store.Store) { _, _ = ss.RemoteCluster().Delete(rc.RemoteId) } + // Clear all SharedChannelUsers sync state - this is critical for test isolation + // The SharedChannelUsers table tracks per-user sync timestamps that can interfere between tests + _, _ = th.SQLStore.GetMaster().Exec("DELETE FROM SharedChannelUsers WHERE 1=1") + + // Clear all SharedChannelAttachments sync state + _, _ = th.SQLStore.GetMaster().Exec("DELETE FROM SharedChannelAttachments WHERE 1=1") + + // Reset sync cursors in any remaining SharedChannelRemotes (before they get deleted) + // This ensures cursors don't persist if deletion fails + _, _ = th.SQLStore.GetMaster().Exec(`UPDATE SharedChannelRemotes SET + LastPostCreateAt = 0, + LastPostCreateId = '', + LastPostUpdateAt = 0, + LastPostId = '', + LastMembersSyncAt = 0 + WHERE 1=1`) + // Remove all channel members from test channels (except the basic team/channel setup) channels, _ := ss.Channel().GetAll(th.BasicTeam.Id) for _, channel := range channels { // Skip direct message and group channels, and skip the default channels if channel.Type != model.ChannelTypeDirect && channel.Type != model.ChannelTypeGroup && channel.Id != th.BasicChannel.Id { - members, _ := ss.Channel().GetMembers(channel.Id, 0, 10000) + members, _ := ss.Channel().GetMembers(model.ChannelMembersGetOptions{ + ChannelID: channel.Id, + }) for _, member := range members { _ = ss.Channel().RemoveMember(th.Context, channel.Id, member.UserId) } @@ -228,12 +276,16 @@ func EnsureCleanState(t *testing.T, th *TestHelper, ss store.Store) { cfg.ConnectedWorkspacesSettings.GlobalUserSyncBatchSize = &defaultBatchSize }) - // Ensure services are running and ready - scsInterface := th.App.Srv().GetSharedChannelSyncService() - if scs, ok := scsInterface.(*sharedchannel.Service); ok { - require.Eventually(t, func() bool { - return scs.Active() - }, 2*time.Second, 100*time.Millisecond, "Shared channel service should be active") + // Restart services and ensure they are running and ready + if scsInterface != nil { + // Restart the shared channel service + _ = scsInterface.Start() + + if scs, ok := scsInterface.(*sharedchannel.Service); ok { + require.Eventually(t, func() bool { + return scs.Active() + }, 5*time.Second, 100*time.Millisecond, "Shared channel service should be active after restart") + } } rcService := th.App.Srv().GetRemoteClusterService() @@ -243,6 +295,6 @@ func EnsureCleanState(t *testing.T, th *TestHelper, ss store.Store) { } require.Eventually(t, func() bool { return rcService.Active() - }, 2*time.Second, 100*time.Millisecond, "Remote cluster service should be active") + }, 5*time.Second, 100*time.Millisecond, "Remote cluster service should be active") } } diff --git a/server/channels/app/shared_channel_test.go b/server/channels/app/shared_channel_test.go index 903c5e742d4..6a76fe5af86 100644 --- a/server/channels/app/shared_channel_test.go +++ b/server/channels/app/shared_channel_test.go @@ -16,6 +16,7 @@ func setupSharedChannels(tb testing.TB) *TestHelper { return SetupConfig(tb, func(cfg *model.Config) { *cfg.ConnectedWorkspacesSettings.EnableRemoteClusterService = true *cfg.ConnectedWorkspacesSettings.EnableSharedChannels = true + cfg.FeatureFlags.EnableSharedChannelsMemberSync = true }) } diff --git a/server/channels/db/migrations/migrations.list b/server/channels/db/migrations/migrations.list index d43281e6c42..0aad4d3c7ce 100644 --- a/server/channels/db/migrations/migrations.list +++ b/server/channels/db/migrations/migrations.list @@ -275,6 +275,8 @@ channels/db/migrations/mysql/000138_add_default_category_name_to_channel.down.sq channels/db/migrations/mysql/000138_add_default_category_name_to_channel.up.sql channels/db/migrations/mysql/000139_remoteclusters_add_last_global_user_sync_at.down.sql channels/db/migrations/mysql/000139_remoteclusters_add_last_global_user_sync_at.up.sql +channels/db/migrations/mysql/000140_add_lastmemberssyncat_to_sharedchannelremotes.down.sql +channels/db/migrations/mysql/000140_add_lastmemberssyncat_to_sharedchannelremotes.up.sql channels/db/migrations/postgres/000001_create_teams.down.sql channels/db/migrations/postgres/000001_create_teams.up.sql channels/db/migrations/postgres/000002_create_team_members.down.sql @@ -551,3 +553,5 @@ channels/db/migrations/postgres/000138_add_default_category_name_to_channel.down channels/db/migrations/postgres/000138_add_default_category_name_to_channel.up.sql channels/db/migrations/postgres/000139_remoteclusters_add_last_global_user_sync_at.down.sql channels/db/migrations/postgres/000139_remoteclusters_add_last_global_user_sync_at.up.sql +channels/db/migrations/postgres/000140_add_lastmemberssyncat_to_sharedchannelremotes.down.sql +channels/db/migrations/postgres/000140_add_lastmemberssyncat_to_sharedchannelremotes.up.sql diff --git a/server/channels/db/migrations/mysql/000140_add_lastmemberssyncat_to_sharedchannelremotes.down.sql b/server/channels/db/migrations/mysql/000140_add_lastmemberssyncat_to_sharedchannelremotes.down.sql new file mode 100644 index 00000000000..ca804f16db5 --- /dev/null +++ b/server/channels/db/migrations/mysql/000140_add_lastmemberssyncat_to_sharedchannelremotes.down.sql @@ -0,0 +1,29 @@ +SET @preparedStatement = (SELECT IF( + ( + SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS + WHERE table_name = 'SharedChannelRemotes' + AND table_schema = DATABASE() + AND column_name = 'LastMembersSyncAt' + ) > 0, + 'ALTER TABLE SharedChannelRemotes DROP COLUMN LastMembersSyncAt;', + 'SELECT 1' +)); + +PREPARE alterIfExists FROM @preparedStatement; +EXECUTE alterIfExists; +DEALLOCATE PREPARE alterIfExists; + +SET @preparedStatement2 = (SELECT IF( + ( + SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS + WHERE table_name = 'SharedChannelUsers' + AND table_schema = DATABASE() + AND column_name = 'LastMembershipSyncAt' + ) > 0, + 'ALTER TABLE SharedChannelUsers DROP COLUMN LastMembershipSyncAt;', + 'SELECT 1' +)); + +PREPARE alterIfExists2 FROM @preparedStatement2; +EXECUTE alterIfExists2; +DEALLOCATE PREPARE alterIfExists2; \ No newline at end of file diff --git a/server/channels/db/migrations/mysql/000140_add_lastmemberssyncat_to_sharedchannelremotes.up.sql b/server/channels/db/migrations/mysql/000140_add_lastmemberssyncat_to_sharedchannelremotes.up.sql new file mode 100644 index 00000000000..66433b228da --- /dev/null +++ b/server/channels/db/migrations/mysql/000140_add_lastmemberssyncat_to_sharedchannelremotes.up.sql @@ -0,0 +1,29 @@ +SET @preparedStatement = (SELECT IF( + ( + SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS + WHERE table_name = 'SharedChannelRemotes' + AND table_schema = DATABASE() + AND column_name = 'LastMembersSyncAt' + ) > 0, + 'SELECT 1', + 'ALTER TABLE SharedChannelRemotes ADD LastMembersSyncAt bigint DEFAULT 0;' +)); + +PREPARE alterIfNotExists FROM @preparedStatement; +EXECUTE alterIfNotExists; +DEALLOCATE PREPARE alterIfNotExists; + +SET @preparedStatement2 = (SELECT IF( + ( + SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS + WHERE table_name = 'SharedChannelUsers' + AND table_schema = DATABASE() + AND column_name = 'LastMembershipSyncAt' + ) > 0, + 'SELECT 1', + 'ALTER TABLE SharedChannelUsers ADD LastMembershipSyncAt bigint DEFAULT 0;' +)); + +PREPARE alterIfNotExists2 FROM @preparedStatement2; +EXECUTE alterIfNotExists2; +DEALLOCATE PREPARE alterIfNotExists2; \ No newline at end of file diff --git a/server/channels/db/migrations/postgres/000140_add_lastmemberssyncat_to_sharedchannelremotes.down.sql b/server/channels/db/migrations/postgres/000140_add_lastmemberssyncat_to_sharedchannelremotes.down.sql new file mode 100644 index 00000000000..78b6da1c7e0 --- /dev/null +++ b/server/channels/db/migrations/postgres/000140_add_lastmemberssyncat_to_sharedchannelremotes.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE sharedchannelremotes DROP COLUMN IF EXISTS lastmemberssyncat; +ALTER TABLE sharedchannelusers DROP COLUMN IF EXISTS lastmembershipsyncat; \ No newline at end of file diff --git a/server/channels/db/migrations/postgres/000140_add_lastmemberssyncat_to_sharedchannelremotes.up.sql b/server/channels/db/migrations/postgres/000140_add_lastmemberssyncat_to_sharedchannelremotes.up.sql new file mode 100644 index 00000000000..5f3ad38b852 --- /dev/null +++ b/server/channels/db/migrations/postgres/000140_add_lastmemberssyncat_to_sharedchannelremotes.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE sharedchannelremotes ADD COLUMN IF NOT EXISTS lastmemberssyncat bigint DEFAULT 0; +ALTER TABLE sharedchannelusers ADD COLUMN IF NOT EXISTS lastmembershipsyncat bigint DEFAULT 0; \ No newline at end of file diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index ba46a9c0d88..a3ad8e2ce88 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -2081,11 +2081,11 @@ func (s *RetryLayerChannelStore) GetMemberLastViewedAt(ctx context.Context, chan } -func (s *RetryLayerChannelStore) GetMembers(channelID string, offset int, limit int) (model.ChannelMembers, error) { +func (s *RetryLayerChannelStore) GetMembers(opts model.ChannelMembersGetOptions) (model.ChannelMembers, error) { tries := 0 for { - result, err := s.ChannelStore.GetMembers(channelID, offset, limit) + result, err := s.ChannelStore.GetMembers(opts) if err == nil { return result, nil } @@ -11642,6 +11642,27 @@ func (s *RetryLayerSharedChannelStore) GetSingleUser(userID string, channelID st } +func (s *RetryLayerSharedChannelStore) GetUserChanges(userID string, channelID string, afterTime int64) ([]*model.SharedChannelUser, error) { + + tries := 0 + for { + result, err := s.SharedChannelStore.GetUserChanges(userID, channelID, afterTime) + if err == nil { + return result, nil + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerSharedChannelStore) GetUsersForSync(filter model.GetUsersForSyncFilter) ([]*model.User, error) { tries := 0 @@ -11894,6 +11915,48 @@ func (s *RetryLayerSharedChannelStore) UpdateRemoteCursor(id string, cursor mode } +func (s *RetryLayerSharedChannelStore) UpdateRemoteMembershipCursor(id string, syncTime int64) error { + + tries := 0 + for { + err := s.SharedChannelStore.UpdateRemoteMembershipCursor(id, syncTime) + if err == nil { + return nil + } + if !isRepeatableError(err) { + return err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + +func (s *RetryLayerSharedChannelStore) UpdateUserLastMembershipSyncAt(userID string, channelID string, remoteID string, syncTime int64) error { + + tries := 0 + for { + err := s.SharedChannelStore.UpdateUserLastMembershipSyncAt(userID, channelID, remoteID, syncTime) + if err == nil { + return nil + } + if !isRepeatableError(err) { + return err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerSharedChannelStore) UpdateUserLastSyncAt(userID string, channelID string, remoteID string) error { tries := 0 diff --git a/server/channels/store/sqlstore/channel_store.go b/server/channels/store/sqlstore/channel_store.go index c12fb586c09..2cb28ef352d 100644 --- a/server/channels/store/sqlstore/channel_store.go +++ b/server/channels/store/sqlstore/channel_store.go @@ -2070,22 +2070,34 @@ func (s SqlChannelStore) PatchMultipleMembersNotifyProps(members []*model.Channe return updated, nil } -func (s SqlChannelStore) GetMembers(channelID string, offset, limit int) (model.ChannelMembers, error) { - sql, args, err := s.channelMembersForTeamWithSchemeSelectQuery. +func (s SqlChannelStore) GetMembers(opts model.ChannelMembersGetOptions) (model.ChannelMembers, error) { + query := s.channelMembersForTeamWithSchemeSelectQuery. Where(sq.Eq{ - "ChannelId": channelID, - }). - Limit(uint64(limit)). - Offset(uint64(offset)). - ToSql() + "ChannelId": opts.ChannelID, + }) + + if opts.UpdatedAfter > 0 { + query = query.Where(sq.Gt{"ChannelMembers.LastUpdateAt": opts.UpdatedAfter}) + query = query.OrderBy("ChannelMembers.LastUpdateAt") + } + + if opts.Limit > 0 { + query = query.Limit(uint64(opts.Limit)) + } + + if opts.Offset > 0 { + query = query.Offset(uint64(opts.Offset)) + } + + sql, args, err := query.ToSql() if err != nil { - return nil, errors.Wrapf(err, "GetMember_ToSql ChannelID=%s", channelID) + return nil, errors.Wrapf(err, "GetMember_ToSql ChannelID=%s", opts.ChannelID) } dbMembers := channelMemberWithSchemeRolesList{} err = s.GetReplica().Select(&dbMembers, sql, args...) if err != nil { - return nil, errors.Wrapf(err, "failed to get ChannelMembers with channelId=%s", channelID) + return nil, errors.Wrapf(err, "failed to get ChannelMembers with channelId=%s", opts.ChannelID) } return dbMembers.ToModel(), nil diff --git a/server/channels/store/sqlstore/shared_channel_store.go b/server/channels/store/sqlstore/shared_channel_store.go index 415e5da8521..625e408be60 100644 --- a/server/channels/store/sqlstore/shared_channel_store.go +++ b/server/channels/store/sqlstore/shared_channel_store.go @@ -417,6 +417,7 @@ func sharedChannelRemoteFields(prefix string) []string { "COALESCE(" + prefix + "LastPostCreateID,'') AS LastPostCreateID", prefix + "LastPostUpdateAt", "COALESCE(" + prefix + "LastPostId,'') AS LastPostUpdateID", + prefix + "LastMembersSyncAt", } } @@ -708,6 +709,7 @@ func sharedChannelUserFields(prefix string) []string { prefix + "RemoteId", prefix + "CreateAt", prefix + "LastSyncAt", + prefix + "LastMembershipSyncAt", } } @@ -720,7 +722,7 @@ func (s SqlSharedChannelStore) SaveUser(scUser *model.SharedChannelUser) (*model query, args, err := s.getQueryBuilder().Insert("SharedChannelUsers"). Columns(sharedChannelUserFields("")...). - Values(scUser.Id, scUser.UserId, scUser.ChannelId, scUser.RemoteId, scUser.CreateAt, scUser.LastSyncAt). + Values(scUser.Id, scUser.UserId, scUser.ChannelId, scUser.RemoteId, scUser.CreateAt, scUser.LastSyncAt, scUser.LastMembershipSyncAt). ToSql() if err != nil { return nil, errors.Wrapf(err, "savesharedchanneluser_tosql") @@ -853,6 +855,25 @@ func (s SqlSharedChannelStore) UpdateUserLastSyncAt(userID string, channelID str return nil } +// UpdateUserLastMembershipSyncAt updates the LastMembershipSyncAt timestamp for the specified SharedChannelUser using the provided sync time. +func (s SqlSharedChannelStore) UpdateUserLastMembershipSyncAt(userID string, channelID string, remoteID string, syncTime int64) error { + query := s.getQueryBuilder(). + Update("SharedChannelUsers AS scu"). + Set("LastMembershipSyncAt", sq.Expr("GREATEST(scu.LastMembershipSyncAt, ?)", syncTime)). + Where(sq.Eq{ + "scu.UserId": userID, + "scu.ChannelId": channelID, + "scu.RemoteId": remoteID, + }) + + _, err := s.GetMaster().ExecBuilder(query) + if err != nil { + return fmt.Errorf("failed to update LastMembershipSyncAt for SharedChannelUser with userId=%s, channelId=%s, remoteId=%s: %w", + userID, channelID, remoteID, err) + } + return nil +} + func sharedChannelAttachementFields(prefix string) []string { if prefix != "" && !strings.HasSuffix(prefix, ".") { prefix = prefix + "." diff --git a/server/channels/store/sqlstore/shared_channel_store_membership.go b/server/channels/store/sqlstore/shared_channel_store_membership.go new file mode 100644 index 00000000000..111fbb85e01 --- /dev/null +++ b/server/channels/store/sqlstore/shared_channel_store_membership.go @@ -0,0 +1,66 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package sqlstore + +import ( + "database/sql" + "fmt" + + "github.com/mattermost/mattermost/server/public/model" + sq "github.com/mattermost/squirrel" + "github.com/pkg/errors" +) + +// UpdateRemoteMembershipCursor updates the LastMembersSyncAt timestamp for the specified SharedChannelRemote, +// but only if the new timestamp is greater than the current value. +func (s SqlSharedChannelStore) UpdateRemoteMembershipCursor(id string, syncTime int64) error { + query := s.getQueryBuilder(). + Update("SharedChannelRemotes") + + query = query.Set("LastMembersSyncAt", sq.Expr("GREATEST(LastMembersSyncAt, ?)", syncTime)) + + query = query.Where(sq.Eq{"Id": id}) + + result, err := s.GetMaster().ExecBuilder(query) + if err != nil { + return errors.Wrap(err, "failed to update membership cursor for SharedChannelRemote") + } + + count, err := result.RowsAffected() + if err != nil { + return errors.Wrap(err, "failed to determine rows affected") + } + + if count == 0 { + return fmt.Errorf("id not found: %s", id) + } + + return nil +} + +// GetUserChanges gets all SharedChannelUser changes for a given user, channel after a specific time. +// This is used to detect if there are conflicting membership changes. +func (s SqlSharedChannelStore) GetUserChanges(userID string, channelID string, afterTime int64) ([]*model.SharedChannelUser, error) { + squery, args, err := s.getQueryBuilder(). + Select(sharedChannelUserFields("")...). + From("SharedChannelUsers"). + Where(sq.Eq{"SharedChannelUsers.UserId": userID}). + Where(sq.Eq{"SharedChannelUsers.ChannelId": channelID}). + Where(sq.Gt{"SharedChannelUsers.LastSyncAt": afterTime}). + ToSql() + + if err != nil { + return nil, errors.Wrapf(err, "getsharedchanneluserchanges_tosql") + } + + users := []*model.SharedChannelUser{} + if err := s.GetReplica().Select(&users, squery, args...); err != nil { + if err == sql.ErrNoRows { + return make([]*model.SharedChannelUser, 0), nil + } + return nil, errors.Wrapf(err, "failed to find shared channel user changes with UserId=%s, ChannelId=%s, afterTime=%d", + userID, channelID, afterTime) + } + return users, nil +} diff --git a/server/channels/store/store.go b/server/channels/store/store.go index a928c00fe44..1e02b9793ce 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -232,7 +232,7 @@ type ChannelStore interface { // It replaces existing fields and creates new ones which don't exist. UpdateMemberNotifyProps(channelID, userID string, props map[string]string) (*model.ChannelMember, error) PatchMultipleMembersNotifyProps(members []*model.ChannelMemberIdentifier, notifyProps map[string]string) ([]*model.ChannelMember, error) - GetMembers(channelID string, offset, limit int) (model.ChannelMembers, error) + GetMembers(opts model.ChannelMembersGetOptions) (model.ChannelMembers, error) GetMember(ctx context.Context, channelID string, userID string) (*model.ChannelMember, error) GetMemberLastViewedAt(ctx context.Context, channelID string, userID string) (int64, error) GetChannelMembersTimezones(channelID string) ([]model.StringMap, error) @@ -1014,6 +1014,7 @@ type SharedChannelStore interface { GetRemoteByIds(channelID string, remoteID string) (*model.SharedChannelRemote, error) GetRemotes(offset, limit int, opts model.SharedChannelRemoteFilterOpts) ([]*model.SharedChannelRemote, error) UpdateRemoteCursor(id string, cursor model.GetPostsSinceForSyncCursor) error + UpdateRemoteMembershipCursor(id string, syncTime int64) error DeleteRemote(remoteID string) (bool, error) GetRemotesStatus(channelID string) ([]*model.SharedChannelRemoteStatus, error) @@ -1021,7 +1022,9 @@ type SharedChannelStore interface { GetSingleUser(userID string, channelID string, remoteID string) (*model.SharedChannelUser, error) GetUsersForUser(userID string) ([]*model.SharedChannelUser, error) GetUsersForSync(filter model.GetUsersForSyncFilter) ([]*model.User, error) + GetUserChanges(userID string, channelID string, afterTime int64) ([]*model.SharedChannelUser, error) UpdateUserLastSyncAt(userID string, channelID string, remoteID string) error + UpdateUserLastMembershipSyncAt(userID string, channelID string, remoteID string, syncTime int64) error SaveAttachment(remote *model.SharedChannelAttachment) (*model.SharedChannelAttachment, error) UpsertAttachment(remote *model.SharedChannelAttachment) (string, error) diff --git a/server/channels/store/storetest/channel_store.go b/server/channels/store/storetest/channel_store.go index 3d0ff7207e0..0adf50b341e 100644 --- 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) diff --git a/server/channels/store/storetest/group_store.go b/server/channels/store/storetest/group_store.go index 1a7c527e5b8..1c0ec0be78b 100644 --- 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 diff --git a/server/channels/store/storetest/mocks/ChannelStore.go b/server/channels/store/storetest/mocks/ChannelStore.go index 4e2c7f30268..e116e430ab9 100644 --- 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) } diff --git a/server/channels/store/storetest/mocks/SharedChannelStore.go b/server/channels/store/storetest/mocks/SharedChannelStore.go index 87120a008c1..b701767f882 100644 --- 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) diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index 90450bfbbd0..97c94ae355c 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -1729,10 +1729,10 @@ func (s *TimerLayerChannelStore) GetMemberLastViewedAt(ctx context.Context, chan return result, err } -func (s *TimerLayerChannelStore) GetMembers(channelID string, offset int, limit int) (model.ChannelMembers, error) { +func (s *TimerLayerChannelStore) GetMembers(opts model.ChannelMembersGetOptions) (model.ChannelMembers, error) { start := time.Now() - result, err := s.ChannelStore.GetMembers(channelID, offset, limit) + result, err := s.ChannelStore.GetMembers(opts) elapsed := float64(time.Since(start)) / float64(time.Second) if s.Root.Metrics != nil { @@ -9173,6 +9173,22 @@ func (s *TimerLayerSharedChannelStore) GetSingleUser(userID string, channelID st return result, err } +func (s *TimerLayerSharedChannelStore) GetUserChanges(userID string, channelID string, afterTime int64) ([]*model.SharedChannelUser, error) { + start := time.Now() + + result, err := s.SharedChannelStore.GetUserChanges(userID, channelID, afterTime) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("SharedChannelStore.GetUserChanges", success, elapsed) + } + return result, err +} + func (s *TimerLayerSharedChannelStore) GetUsersForSync(filter model.GetUsersForSyncFilter) ([]*model.User, error) { start := time.Now() @@ -9365,6 +9381,38 @@ func (s *TimerLayerSharedChannelStore) UpdateRemoteCursor(id string, cursor mode return err } +func (s *TimerLayerSharedChannelStore) UpdateRemoteMembershipCursor(id string, syncTime int64) error { + start := time.Now() + + err := s.SharedChannelStore.UpdateRemoteMembershipCursor(id, syncTime) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("SharedChannelStore.UpdateRemoteMembershipCursor", success, elapsed) + } + return err +} + +func (s *TimerLayerSharedChannelStore) UpdateUserLastMembershipSyncAt(userID string, channelID string, remoteID string, syncTime int64) error { + start := time.Now() + + err := s.SharedChannelStore.UpdateUserLastMembershipSyncAt(userID, channelID, remoteID, syncTime) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("SharedChannelStore.UpdateUserLastMembershipSyncAt", success, elapsed) + } + return err +} + func (s *TimerLayerSharedChannelStore) UpdateUserLastSyncAt(userID string, channelID string, remoteID string) error { start := time.Now() diff --git a/server/platform/services/remotecluster/service.go b/server/platform/services/remotecluster/service.go index b60e59579f6..e21f2f021ed 100644 --- a/server/platform/services/remotecluster/service.go +++ b/server/platform/services/remotecluster/service.go @@ -290,7 +290,7 @@ func (rcs *Service) pause() { rcs.server.Log().Debug("Remote Cluster Service inactive") } -// SetActive forces the service to be active or inactive +// SetActive forces the service to be active or inactive for testing func (rcs *Service) SetActive(active bool) { rcs.mux.Lock() defer rcs.mux.Unlock() diff --git a/server/platform/services/sharedchannel/channelinvite.go b/server/platform/services/sharedchannel/channelinvite.go index 363367aeb09..ebf16930080 100644 --- a/server/platform/services/sharedchannel/channelinvite.go +++ b/server/platform/services/sharedchannel/channelinvite.go @@ -89,6 +89,7 @@ func (scs *Service) SendChannelInvite(channel *model.Channel, userId string, rc RemoteId: rc.RemoteId, IsInviteAccepted: true, IsInviteConfirmed: false, + LastMembersSyncAt: 0, } if _, err = scs.server.GetStore().SharedChannel().SaveRemote(scr); err != nil { scs.sendEphemeralPost(channel.Id, userId, fmt.Sprintf("Error saving channel invite for %s: %v", rc.DisplayName, err)) @@ -134,6 +135,7 @@ func (scs *Service) SendChannelInvite(channel *model.Channel, userId string, rc } curTime := model.GetMillis() + var sharedChannelRemote *model.SharedChannelRemote if existingScr != nil { if existingScr.DeleteAt == 0 && existingScr.IsInviteConfirmed { // the shared channel remote exists and is not @@ -153,6 +155,7 @@ func (scs *Service) SendChannelInvite(channel *model.Channel, userId string, rc scs.sendEphemeralPost(channel.Id, userId, fmt.Sprintf("Error confirming channel invite for %s: %v", rc.DisplayName, sErr)) return } + sharedChannelRemote = existingScr } else { // the shared channel remote doesn't exists, so we create it scr := &model.SharedChannelRemote{ @@ -163,15 +166,26 @@ func (scs *Service) SendChannelInvite(channel *model.Channel, userId string, rc IsInviteConfirmed: true, LastPostCreateAt: curTime, LastPostUpdateAt: curTime, + LastMembersSyncAt: 0, } if _, err = scs.server.GetStore().SharedChannel().SaveRemote(scr); err != nil { scs.sendEphemeralPost(channel.Id, userId, fmt.Sprintf("Error confirming channel invite for %s: %v", rc.DisplayName, err)) return } + sharedChannelRemote = scr } scs.NotifyChannelChanged(sc.ChannelId) scs.sendEphemeralPost(channel.Id, userId, fmt.Sprintf("`%s` has been added to channel.", rc.DisplayName)) + + // Sync all channel members to the remote now that the remote entry exists + if syncErr := scs.SyncAllChannelMembers(sc.ChannelId, rc.RemoteId, sharedChannelRemote); syncErr != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Failed to sync channel members after invite confirmation", + mlog.String("channel_id", sc.ChannelId), + mlog.String("remote_id", rc.RemoteId), + mlog.Err(syncErr), + ) + } } if rc.IsPlugin() { @@ -311,6 +325,15 @@ func (scs *Service) onReceiveChannelInvite(msg model.RemoteClusterMsg, rc *model if _, err := scs.server.GetStore().SharedChannel().UpdateRemote(existingScr); err != nil { return fmt.Errorf("cannot restore deleted shared channel remote (channel_id=%s): %w", invite.ChannelId, err) } + + // Sync local channel members to the remote after restoring the shared channel + if syncErr := scs.SyncAllChannelMembers(channel.Id, rc.RemoteId, existingScr); syncErr != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Failed to sync local channel members after restoring shared channel", + mlog.String("channel_id", channel.Id), + mlog.String("remote_id", rc.RemoteId), + mlog.Err(syncErr), + ) + } } else { creatorID := channel.CreatorId if creatorID == "" { @@ -325,6 +348,7 @@ func (scs *Service) onReceiveChannelInvite(msg model.RemoteClusterMsg, rc *model RemoteId: rc.RemoteId, LastPostCreateAt: model.GetMillis(), LastPostUpdateAt: model.GetMillis(), + LastMembersSyncAt: 0, } if _, err := scs.server.GetStore().SharedChannel().SaveRemote(scr); err != nil { @@ -336,6 +360,15 @@ func (scs *Service) onReceiveChannelInvite(msg model.RemoteClusterMsg, rc *model scs.server.GetStore().SharedChannel().Delete(sharedChannel.ChannelId) return fmt.Errorf("cannot create shared channel remote (channel_id=%s): %w", invite.ChannelId, err) } + + // Sync local channel members to the remote after accepting the invitation + if syncErr := scs.SyncAllChannelMembers(channel.Id, rc.RemoteId, scr); syncErr != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Failed to sync local channel members after accepting invitation", + mlog.String("channel_id", channel.Id), + mlog.String("remote_id", rc.RemoteId), + mlog.Err(syncErr), + ) + } } return nil } diff --git a/server/platform/services/sharedchannel/channelinvite_test.go b/server/platform/services/sharedchannel/channelinvite_test.go index 8f0f94f69c4..f85cf708f62 100644 --- a/server/platform/services/sharedchannel/channelinvite_test.go +++ b/server/platform/services/sharedchannel/channelinvite_test.go @@ -29,6 +29,18 @@ var ( mockTypeContext = mock.MatchedBy(func(ctx context.Context) bool { return true }) ) +// setupMockServerWithConfig sets up the standard mocks that all tests need +func setupMockServerWithConfig(mockServer *MockServerIface) { + // Mock Config for feature flag check - disable membership sync to avoid complex mocking + mockConfig := model.Config{} + mockConfig.SetDefaults() + mockConfig.FeatureFlags.EnableSharedChannelsMemberSync = false + mockServer.On("Config").Return(&mockConfig) + + // Mock GetRemoteClusterService for feature flag check + mockServer.On("GetRemoteClusterService").Return(nil) +} + func TestOnReceiveChannelInvite(t *testing.T) { t.Run("when msg payload is empty, it does nothing", func(t *testing.T) { mockServer := &MockServerIface{} @@ -92,6 +104,8 @@ func TestOnReceiveChannelInvite(t *testing.T) { mockStore.On("SharedChannel").Return(&mockSharedChannelStore) mockServer.On("GetStore").Return(mockStore) + setupMockServerWithConfig(mockServer) + createPostPermission := model.ChannelModeratedPermissionsMap[model.PermissionCreatePost.Id] createReactionPermission := model.ChannelModeratedPermissionsMap[model.PermissionAddReaction.Id] updateMap := model.ChannelModeratedRolesPatch{ @@ -216,6 +230,8 @@ func TestOnReceiveChannelInvite(t *testing.T) { mockStore.On("SharedChannel").Return(&mockSharedChannelStore) mockServer.On("GetStore").Return(mockStore) + setupMockServerWithConfig(mockServer) + defer mockApp.AssertExpectations(t) err = scs.onReceiveChannelInvite(msg, remoteCluster, nil) @@ -351,6 +367,7 @@ func TestOnReceiveChannelInvite(t *testing.T) { mockServer = scs.server.(*MockServerIface) mockServer.On("GetStore").Return(mockStore) + setupMockServerWithConfig(mockServer) mockApp.On("GetOrCreateDirectChannel", mockTypeReqContext, mockTypeString, mockTypeString, mock.AnythingOfType("model.ChannelOption")). Return(channel, nil).Maybe() diff --git a/server/platform/services/sharedchannel/membership.go b/server/platform/services/sharedchannel/membership.go new file mode 100644 index 00000000000..8903b217f66 --- /dev/null +++ b/server/platform/services/sharedchannel/membership.go @@ -0,0 +1,409 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package sharedchannel + +import ( + "context" + "encoding/json" + "fmt" + "time" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/mlog" + "github.com/mattermost/mattermost/server/v8/platform/services/remotecluster" +) + +// isChannelMemberSyncEnabled checks if the feature flag is enabled and remote cluster service is available +func (scs *Service) isChannelMemberSyncEnabled() bool { + featureFlagEnabled := scs.server.Config().FeatureFlags.EnableSharedChannelsMemberSync + remoteClusterService := scs.server.GetRemoteClusterService() + return featureFlagEnabled && remoteClusterService != nil +} + +// queueMembershipSyncTask creates and queues a task to synchronize channel membership changes +func (scs *Service) queueMembershipSyncTask(channelID, userID, remoteID string, syncMsg *model.SyncMsg, retryMsg *model.SyncMsg) { + task := newSyncTask(channelID, userID, remoteID, syncMsg, retryMsg) + task.schedule = time.Now().Add(NotifyMinimumDelay) + + scs.addTask(task) +} + +// HandleMembershipChange is called when users are added or removed from a shared channel. +// It creates a task to notify all remote clusters about the membership change. +func (scs *Service) HandleMembershipChange(channelID, userID string, isAdd bool, remoteID string) { + if !scs.isChannelMemberSyncEnabled() { + return + } + + // Create timestamp for consistent usage + changeTime := model.GetMillis() + + // Create membership change info + syncMsg := model.NewSyncMsg(channelID) + syncMsg.MembershipChanges = []*model.MembershipChangeMsg{ + { + ChannelId: channelID, + UserId: userID, + IsAdd: isAdd, + RemoteId: remoteID, // which remote initiated this change + ChangeTime: changeTime, + }, + } + + // Queue the membership change task + scs.queueMembershipSyncTask(channelID, userID, "", syncMsg, nil) +} + +// HandleMembershipBatchChange is called to process a batch of membership changes for a shared channel. +// It creates a task to notify all remote clusters about the batch membership changes. +func (scs *Service) HandleMembershipBatchChange(channelID string, userIDs []string, isAdd bool, remoteID string) { + if !scs.isChannelMemberSyncEnabled() { + return + } + + if len(userIDs) == 0 { + return + } + + // Create timestamp for consistent usage + changeTime := model.GetMillis() + + // Create sync message with membership changes + syncMsg := model.NewSyncMsg(channelID) + syncMsg.MembershipChanges = make([]*model.MembershipChangeMsg, 0, len(userIDs)) + + // Add each user to the batch + for _, userID := range userIDs { + syncMsg.MembershipChanges = append(syncMsg.MembershipChanges, &model.MembershipChangeMsg{ + ChannelId: channelID, + UserId: userID, + IsAdd: isAdd, + RemoteId: remoteID, + ChangeTime: changeTime, + }) + } + + // Queue the batch membership sync task + scs.queueMembershipSyncTask(channelID, "", "", syncMsg, nil) +} + +// SyncAllChannelMembers synchronizes all channel members to a specific remote. +// This is typically called when a channel is first shared with a remote cluster. +// If remote is provided, it will be used instead of fetching from the database. +func (scs *Service) SyncAllChannelMembers(channelID string, remoteID string, remote *model.SharedChannelRemote) error { + if !scs.isChannelMemberSyncEnabled() { + return nil + } + + // Verify the channel exists and is shared + if _, err := scs.server.GetStore().SharedChannel().Get(channelID); err != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceDebug, "Failed to get shared channel", + mlog.String("channel_id", channelID), + mlog.Err(err), + ) + return fmt.Errorf("failed to get shared channel %s: %w", channelID, err) + } + + // Get the remote to ensure it exists (if not provided) + if remote == nil { + var err error + remote, err = scs.server.GetStore().SharedChannel().GetRemoteByIds(channelID, remoteID) + if err != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceDebug, "Failed to get remote", + mlog.String("channel_id", channelID), + mlog.String("remote_id", remoteID), + mlog.Err(err), + ) + return fmt.Errorf("failed to get remote for channel %s: %w", channelID, err) + } + } + + // Use offset-based pagination to handle channels with many members + // This ensures we don't skip members when multiple members have the same LastUpdateAt timestamp + maxPerPage := scs.GetMemberSyncBatchSize() + var allMembers model.ChannelMembers + lastSyncAt := remote.LastMembersSyncAt + offset := 0 + + // Process members incrementally with offset-based pagination + for { + opts := model.ChannelMembersGetOptions{ + ChannelID: channelID, + UpdatedAfter: lastSyncAt, + Limit: maxPerPage, + Offset: offset, + } + + members, err1 := scs.server.GetStore().Channel().GetMembers(opts) + if err1 != nil { + return fmt.Errorf("failed to get members for channel %s: %w", channelID, err1) + } + + if len(members) == 0 { + break // No more members to process + } + + // Add to our collection + allMembers = append(allMembers, members...) + + // Log progress when processing large channels + if len(allMembers)%1000 == 0 { + scs.server.Log().Log(mlog.LvlSharedChannelServiceDebug, "Processing channel members in batches", + mlog.String("channel_id", channelID), + mlog.String("remote_id", remoteID), + mlog.Int("processed_so_far", len(allMembers)), + ) + } + + if len(members) < maxPerPage { + break // Last page + } + + // Move to next page + offset += maxPerPage + } + + if len(allMembers) == 0 { + scs.server.Log().Log(mlog.LvlSharedChannelServiceDebug, "No members to sync for channel", + mlog.String("channel_id", channelID), + mlog.String("remote_id", remoteID), + ) + return nil + } + + scs.server.Log().Log(mlog.LvlSharedChannelServiceDebug, "Syncing all channel members", + mlog.String("channel_id", channelID), + mlog.String("remote_id", remoteID), + mlog.Int("member_count", len(allMembers)), + ) + + // Get batch size from config + batchSize := scs.GetMemberSyncBatchSize() + + // For small channels, queue individual membership changes + if len(allMembers) <= batchSize { + return scs.syncMembersIndividually(channelID, remoteID, allMembers, remote) + } + + // For larger channels, use batch processing + return scs.syncMembersInBatches(channelID, remoteID, allMembers, remote) +} + +// syncMembersIndividually processes each member individually +// This is more efficient for small channels +func (scs *Service) syncMembersIndividually(channelID, remoteID string, members model.ChannelMembers, remote *model.SharedChannelRemote) error { + // Queue individual membership changes for each member + for _, member := range members { + // Queue membership change for this user (isAdd=true) + scs.HandleMembershipChange(channelID, member.UserId, true, "") + } + + return nil +} + +// syncMembersInBatches processes members in batches for greater efficiency +// This is better for channels with many members +func (scs *Service) syncMembersInBatches(channelID, remoteID string, members model.ChannelMembers, remote *model.SharedChannelRemote) error { + // Get batch size from config + batchSize := scs.GetMemberSyncBatchSize() + + for i := 0; i < len(members); i += batchSize { + end := i + batchSize + if end > len(members) { + end = len(members) + } + + // Create a batch of members + batchMembers := members[i:end] + + // Extract user IDs from the batch + userIDs := make([]string, len(batchMembers)) + for j, member := range batchMembers { + userIDs[j] = member.UserId + } + + // Use the batch handling function to queue the changes + scs.HandleMembershipBatchChange(channelID, userIDs, true, "") + } + + return nil +} + +// processMembershipChange processes a channel membership change task. +// It determines which remotes should receive the update and creates tasks for each. +func (scs *Service) processMembershipChange(syncMsg *model.SyncMsg) { + if len(syncMsg.MembershipChanges) == 0 { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Invalid membership change task - no membership changes", + mlog.String("channel_id", syncMsg.ChannelId), + ) + return + } + + // Get the shared channel (to verify it exists) + _, err := scs.server.GetStore().SharedChannel().Get(syncMsg.ChannelId) + if err != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Failed to get shared channel for membership change", + mlog.String("channel_id", syncMsg.ChannelId), + mlog.Int("change_count", len(syncMsg.MembershipChanges)), + mlog.Err(err), + ) + return + } + + // Get all remotes for this channel + remotes, err := scs.server.GetStore().SharedChannel().GetRemotes(0, 999999, model.SharedChannelRemoteFilterOpts{ + ChannelId: syncMsg.ChannelId, + }) + if err != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Failed to get shared channel remotes for membership change", + mlog.String("channel_id", syncMsg.ChannelId), + mlog.Err(err), + ) + return + } + + // Always use batch processing for consistency (works for single or multiple changes) + scs.syncMembershipBatchToRemotes(syncMsg, remotes) +} + +// syncMembershipBatchToRemotes synchronizes membership changes (single or batch) with remote clusters. +func (scs *Service) syncMembershipBatchToRemotes(syncMsg *model.SyncMsg, remotes []*model.SharedChannelRemote) { + if len(syncMsg.MembershipChanges) == 0 { + return + } + + // Get the initiating remote ID from the first change (all should be the same) + initiatingRemoteId := "" + if len(syncMsg.MembershipChanges) > 0 { + initiatingRemoteId = syncMsg.MembershipChanges[0].RemoteId + } + + // Send to all remotes except the one that initiated this change + for _, remote := range remotes { + // Skip the remote that initiated this change to prevent loops + if remote.RemoteId == initiatingRemoteId { + continue + } + + // Get the remote cluster + rc, err := scs.server.GetStore().RemoteCluster().Get(remote.RemoteId, false) + if err != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Failed to get remote cluster for batch membership sync", + mlog.String("remote_id", remote.RemoteId), + mlog.String("channel_id", syncMsg.ChannelId), + mlog.Err(err), + ) + continue + } + + // Create a copy of the sync message to potentially add user profiles + enrichedSyncMsg := &model.SyncMsg{ + Id: syncMsg.Id, + ChannelId: syncMsg.ChannelId, + MembershipChanges: syncMsg.MembershipChanges, + Users: make(map[string]*model.User), + } + + // Add user profiles for all users being added + for _, change := range syncMsg.MembershipChanges { + if change.IsAdd { + user, pErr := scs.server.GetStore().User().Get(context.Background(), change.UserId) + if pErr != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceWarn, "Failed to get user for batch membership sync", + mlog.String("user_id", change.UserId), + mlog.String("channel_id", syncMsg.ChannelId), + mlog.String("remote_id", remote.RemoteId), + mlog.Err(pErr), + ) + continue + } + + // Check if user profile needs to be synced + doSync, _, sErr := scs.shouldUserSync(user, syncMsg.ChannelId, rc) + if sErr == nil && doSync { + enrichedSyncMsg.Users[user.Id] = user + } + } + } + + // Send message using the existing remote cluster framework + payload, err := json.Marshal(enrichedSyncMsg) + if err != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Failed to marshal batch membership message", + mlog.String("remote_id", remote.RemoteId), + mlog.String("channel_id", syncMsg.ChannelId), + mlog.Err(err), + ) + continue + } + + msg := model.RemoteClusterMsg{ + Id: model.NewId(), + Topic: TopicChannelMembership, + CreateAt: model.GetMillis(), + Payload: payload, + } + + ctx, cancel := context.WithTimeout(context.Background(), remotecluster.SendTimeout) + defer cancel() + + // Define a callback function + callback := func(msg model.RemoteClusterMsg, rc *model.RemoteCluster, resp *remotecluster.Response, err error) { + if err != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Error sending batch membership changes to remote", + mlog.String("remote", remote.RemoteId), + mlog.String("channel_id", syncMsg.ChannelId), + mlog.Int("change_count", len(syncMsg.MembershipChanges)), + mlog.Err(err), + ) + return + } + + if resp != nil && resp.Err != "" { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Remote error when processing batch membership changes", + mlog.String("remote", remote.RemoteId), + mlog.String("channel_id", syncMsg.ChannelId), + mlog.String("remote_error", resp.Err), + ) + return + } + + // Update sync timestamps + for _, change := range syncMsg.MembershipChanges { + if err := scs.server.GetStore().SharedChannel().UpdateUserLastMembershipSyncAt(change.UserId, change.ChannelId, remote.RemoteId, change.ChangeTime); err != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Failed to update user membership sync timestamp in batch", + mlog.String("user_id", change.UserId), + mlog.Err(err), + ) + } + } + + // Update the cursor with the latest change time + var maxChangeTime int64 + for _, change := range syncMsg.MembershipChanges { + if change.ChangeTime > maxChangeTime { + maxChangeTime = change.ChangeTime + } + } + + if err := scs.updateMembershipSyncCursor(syncMsg.ChannelId, remote.RemoteId, maxChangeTime); err != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Failed to update membership sync cursor for batch", + mlog.String("remote_id", remote.RemoteId), + mlog.String("channel_id", syncMsg.ChannelId), + mlog.Err(err), + ) + } + } + + err = scs.server.GetRemoteClusterService().SendMsg(ctx, msg, rc, callback) + + if err != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Failed to send batch membership changes to remote", + mlog.String("remote_id", remote.RemoteId), + mlog.String("channel_id", syncMsg.ChannelId), + mlog.Err(err), + ) + } + } +} diff --git a/server/platform/services/sharedchannel/membership_recv.go b/server/platform/services/sharedchannel/membership_recv.go new file mode 100644 index 00000000000..479b4d18f26 --- /dev/null +++ b/server/platform/services/sharedchannel/membership_recv.go @@ -0,0 +1,213 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package sharedchannel + +import ( + "fmt" + "strings" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/mlog" + "github.com/mattermost/mattermost/server/public/shared/request" + "github.com/mattermost/mattermost/server/v8/platform/services/remotecluster" +) + +// checkMembershipConflict checks if there are newer changes that would conflict with this one +// Returns true if this change should be skipped due to a conflict +func (scs *Service) checkMembershipConflict(userID, channelID string, changeTime int64) (bool, error) { + conflicts, err := scs.server.GetStore().SharedChannel().GetUserChanges(userID, channelID, changeTime) + if err != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Failed to check for membership change conflicts", + mlog.String("user_id", userID), + mlog.String("channel_id", channelID), + mlog.Err(err), + ) + return false, err + } + + // If there are conflicting operations, the latest one wins + for _, conflict := range conflicts { + if conflict.LastMembershipSyncAt > changeTime { + scs.server.Log().Log(mlog.LvlSharedChannelServiceDebug, "Ignoring older membership change due to conflict", + mlog.String("user_id", userID), + mlog.String("channel_id", channelID), + mlog.Int("change_time", int(changeTime)), + mlog.Int("conflicting_time", int(conflict.LastMembershipSyncAt)), + ) + return true, nil + } + } + + return false, nil +} + +// onReceiveMembershipChanges processes channel membership changes from a remote cluster +func (scs *Service) onReceiveMembershipChanges(syncMsg *model.SyncMsg, rc *model.RemoteCluster, response *remotecluster.Response) error { + // Check if feature flag is enabled + if !scs.server.Config().FeatureFlags.EnableSharedChannelsMemberSync { + return nil + } + + if len(syncMsg.MembershipChanges) == 0 { + return fmt.Errorf("onReceiveMembershipChanges: no membership changes") + } + + // Get the channel to make sure it exists and is shared + channel, err := scs.server.GetStore().Channel().Get(syncMsg.ChannelId, true) + if err != nil { + return fmt.Errorf("cannot get channel for membership changes: %w", err) + } + + // Verify this is a valid shared channel + _, err = scs.server.GetStore().SharedChannel().Get(syncMsg.ChannelId) + if err != nil { + return fmt.Errorf("cannot get shared channel for membership changes: %w", err) + } + + // Calculate the maximum ChangeTime from all changes in the batch + var maxChangeTime int64 + for _, change := range syncMsg.MembershipChanges { + if change.ChangeTime > maxChangeTime { + maxChangeTime = change.ChangeTime + } + } + + // Process each change + var successCount, skipCount, failCount int + + for _, change := range syncMsg.MembershipChanges { + // Check for conflicts + shouldSkip, _ := scs.checkMembershipConflict(change.UserId, change.ChannelId, change.ChangeTime) + if shouldSkip { + skipCount++ + continue + } + + // Process the membership change based on whether it's an add or remove + var processErr error + if change.IsAdd { + scs.server.Log().Log(mlog.LvlSharedChannelServiceDebug, "Adding user to channel from remote cluster", + mlog.String("user_id", change.UserId), + mlog.String("channel_id", change.ChannelId), + mlog.String("remote_id", rc.RemoteId), + ) + processErr = scs.processMemberAdd(change, channel, rc, maxChangeTime) + } else { + scs.server.Log().Log(mlog.LvlSharedChannelServiceDebug, "Removing user from channel from remote cluster", + mlog.String("user_id", change.UserId), + mlog.String("channel_id", change.ChannelId), + mlog.String("remote_id", rc.RemoteId), + ) + processErr = scs.processMemberRemove(change, rc, maxChangeTime) + } + + if processErr != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Failed to process membership change", + mlog.String("user_id", change.UserId), + mlog.String("channel_id", change.ChannelId), + mlog.String("remote_id", rc.RemoteId), + mlog.Bool("is_add", change.IsAdd), + mlog.Err(processErr), + ) + failCount++ + continue + } + + successCount++ + } + + return nil +} + +// processMemberAdd handles adding a user to a channel as part of batch processing +func (scs *Service) processMemberAdd(change *model.MembershipChangeMsg, channel *model.Channel, rc *model.RemoteCluster, maxChangeTime int64) error { + // Get the user if they exist + user, err := scs.server.GetStore().User().Get(request.EmptyContext(scs.server.Log()).Context(), change.UserId) + if err != nil { + return fmt.Errorf("cannot get user for channel add: %w", err) + } + + // Check user permissions for private channels + if channel.Type == model.ChannelTypePrivate { + // Add user to team if needed for private channel + rctx := request.EmptyContext(scs.server.Log()) + appErr := scs.app.AddUserToTeamByTeamId(rctx, channel.TeamId, user) + if appErr != nil { + return fmt.Errorf("cannot add user to team for private channel: %w", appErr) + } + } + + // Use the app layer to add the user to the channel + // This ensures proper processing of all side effects + rctx := request.EmptyContext(scs.server.Log()) + _, appErr := scs.app.AddUserToChannel(rctx, user, channel, false) + if appErr != nil { + // Skip "already added" errors + if appErr.Error() != "api.channel.add_user.to_channel.failed.app_error" && + !strings.Contains(appErr.Error(), "channel_member_exists") { + return fmt.Errorf("cannot add user to channel: %w", appErr) + } + // User is already in the channel, which is fine + } + + // Update the sync status + if syncErr := scs.server.GetStore().SharedChannel().UpdateUserLastMembershipSyncAt(change.UserId, change.ChannelId, rc.RemoteId, maxChangeTime); syncErr != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Failed to update user LastMembershipSyncAt after batch member add", + mlog.String("user_id", change.UserId), + mlog.String("channel_id", change.ChannelId), + mlog.String("remote_id", rc.RemoteId), + mlog.Err(syncErr), + ) + // Continue despite the error - this is not critical + } + + return nil +} + +// processMemberRemove handles removing a user from a channel as part of batch processing +func (scs *Service) processMemberRemove(change *model.MembershipChangeMsg, rc *model.RemoteCluster, maxChangeTime int64) error { + // Get channel so we can use app layer methods properly + channel, err := scs.server.GetStore().Channel().Get(change.ChannelId, true) + if err != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceWarn, "Cannot find channel for member removal", + mlog.String("channel_id", change.ChannelId), + mlog.String("user_id", change.UserId), + mlog.Err(err), + ) + // Continue anyway to update sync status - the channel might be deleted + } + + // Use the app layer's remove user method if channel still exists + if channel != nil { + rctx := request.EmptyContext(scs.server.Log()) + // We use empty string for removerUserId to indicate system-initiated removal + // This also ensures we bypass permission checks intended for user-initiated removals + appErr := scs.app.RemoveUserFromChannel(rctx, change.UserId, "", channel) + if appErr != nil { + // Ignore "not found" errors - the user might already be removed + if !strings.Contains(appErr.Error(), "store.sql_channel.remove_member.missing.app_error") { + scs.server.Log().Log(mlog.LvlSharedChannelServiceWarn, "Error removing user from channel", + mlog.String("channel_id", change.ChannelId), + mlog.String("user_id", change.UserId), + mlog.Err(appErr), + ) + // Continue anyway to update sync status - don't return error here + // to ensure sync status still gets updated + } + } + } + + // Update the sync status + if syncErr := scs.server.GetStore().SharedChannel().UpdateUserLastMembershipSyncAt(change.UserId, change.ChannelId, rc.RemoteId, maxChangeTime); syncErr != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Failed to update user LastMembershipSyncAt after batch member remove", + mlog.String("user_id", change.UserId), + mlog.String("channel_id", change.ChannelId), + mlog.String("remote_id", rc.RemoteId), + mlog.Err(syncErr), + ) + // Continue despite the error - this is not critical + } + + return nil +} diff --git a/server/platform/services/sharedchannel/mock_AppIface_test.go b/server/platform/services/sharedchannel/mock_AppIface_test.go index 9a3105e1054..b6a5afc8a40 100644 --- a/server/platform/services/sharedchannel/mock_AppIface_test.go +++ b/server/platform/services/sharedchannel/mock_AppIface_test.go @@ -513,6 +513,26 @@ func (_m *MockAppIface) Publish(message *model.WebSocketEvent) { _m.Called(message) } +// RemoveUserFromChannel provides a mock function with given fields: c, userID, removerUserId, channel +func (_m *MockAppIface) RemoveUserFromChannel(c request.CTX, userID string, removerUserId string, channel *model.Channel) *model.AppError { + ret := _m.Called(c, userID, removerUserId, channel) + + if len(ret) == 0 { + panic("no return value specified for RemoveUserFromChannel") + } + + var r0 *model.AppError + if rf, ok := ret.Get(0).(func(request.CTX, string, string, *model.Channel) *model.AppError); ok { + r0 = rf(c, userID, removerUserId, channel) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.AppError) + } + } + + return r0 +} + // SaveAndBroadcastStatus provides a mock function with given fields: status func (_m *MockAppIface) SaveAndBroadcastStatus(status *model.Status) { _m.Called(status) diff --git a/server/platform/services/sharedchannel/service.go b/server/platform/services/sharedchannel/service.go index 5f1212bdbda..503d4cbb663 100644 --- a/server/platform/services/sharedchannel/service.go +++ b/server/platform/services/sharedchannel/service.go @@ -24,6 +24,7 @@ const ( TopicSync = "sharedchannel_sync" TopicChannelInvite = "sharedchannel_invite" TopicUploadCreate = "sharedchannel_upload" + TopicChannelMembership = "sharedchannel_membership" TopicGlobalUserSync = "sharedchannel_global_user_sync" MaxRetries = 3 MaxUsersPerSync = 25 @@ -31,6 +32,7 @@ const ( NotifyMinimumDelay = time.Second * 2 MaxUpsertRetries = 25 ProfileImageSyncTimeout = time.Second * 5 + // Default value for MaxMembersPerBatch is defined in config.go as ConnectedWorkspacesSettingsDefaultMemberSyncBatchSize ) // Mocks can be re-generated with `make sharedchannel-mocks`. @@ -58,6 +60,7 @@ type AppIface interface { UserCanSeeOtherUser(c request.CTX, userID string, otherUserId string) (bool, *model.AppError) AddUserToChannel(c request.CTX, user *model.User, channel *model.Channel, skipTeamMemberIntegrityCheck bool) (*model.ChannelMember, *model.AppError) AddUserToTeamByTeamId(c request.CTX, teamId string, user *model.User) *model.AppError + RemoveUserFromChannel(c request.CTX, userID string, removerUserId string, channel *model.Channel) *model.AppError PermanentDeleteChannel(c request.CTX, channel *model.Channel) *model.AppError CreatePost(c request.CTX, post *model.Post, channel *model.Channel, flags model.CreatePostFlags) (savedPost *model.Post, err *model.AppError) UpdatePost(c request.CTX, post *model.Post, updatePostOptions *model.UpdatePostOptions) (*model.Post, *model.AppError) @@ -90,9 +93,10 @@ type Service struct { changeSignal chan struct{} // everything below guarded by `mux` - mux sync.RWMutex - active bool - leaderListenerId string + mux sync.RWMutex + active bool + leaderListenerId string + connectionStateListenerId string done chan struct{} tasks map[string]syncTask @@ -136,6 +140,8 @@ func (scs *Service) Start() error { scs.connectionStateListenerId = rcs.AddConnectionStateListener(scs.onConnectionStateChange) scs.mux.Unlock() + rcs.AddTopicListener(TopicChannelMembership, scs.onReceiveSyncMessage) + scs.onClusterLeaderChange() return nil @@ -220,6 +226,14 @@ func (scs *Service) pause() { scs.server.Log().Debug("Shared Channel Service inactive") } +// GetMemberSyncBatchSize returns the configured batch size for member synchronization +func (scs *Service) GetMemberSyncBatchSize() int { + if scs.server.Config().ConnectedWorkspacesSettings.MemberSyncBatchSize != nil { + return *scs.server.Config().ConnectedWorkspacesSettings.MemberSyncBatchSize + } + return model.ConnectedWorkspacesSettingsDefaultMemberSyncBatchSize +} + // Makes the remote channel to be read-only(announcement mode, only admins can create posts and reactions). func (scs *Service) makeChannelReadOnly(channel *model.Channel) *model.AppError { createPostPermission := model.ChannelModeratedPermissionsMap[model.PermissionCreatePost.Id] @@ -314,17 +328,19 @@ func (scs *Service) scheduleGlobalUserSync(rc *model.RemoteCluster) { }() } -// OnReceiveSyncMessageForTesting exposes onReceiveSyncMessage for testing -func (scs *Service) OnReceiveSyncMessageForTesting(msg model.RemoteClusterMsg, rc *model.RemoteCluster, response *remotecluster.Response) error { - return scs.onReceiveSyncMessage(msg, rc, response) -} - -// GetUserSyncBatchSizeForTesting returns the configured batch size for user syncing (exported for testing) -func (scs *Service) GetUserSyncBatchSizeForTesting() int { - return scs.getGlobalUserSyncBatchSize() +// HasPendingTasksForTesting returns true if there are pending sync tasks in the queue +func (scs *Service) HasPendingTasksForTesting() bool { + scs.mux.RLock() + defer scs.mux.RUnlock() + return len(scs.tasks) > 0 } // HandleSyncAllUsersForTesting exposes syncAllUsers for testing func (scs *Service) HandleSyncAllUsersForTesting(rc *model.RemoteCluster) error { return scs.syncAllUsers(rc) } + +// OnReceiveSyncMessageForTesting exposes onReceiveSyncMessage for testing +func (scs *Service) OnReceiveSyncMessageForTesting(msg model.RemoteClusterMsg, rc *model.RemoteCluster, response *remotecluster.Response) error { + return scs.onReceiveSyncMessage(msg, rc, response) +} diff --git a/server/platform/services/sharedchannel/service_api.go b/server/platform/services/sharedchannel/service_api.go index bddbd9e36a2..f0f989820ed 100644 --- a/server/platform/services/sharedchannel/service_api.go +++ b/server/platform/services/sharedchannel/service_api.go @@ -277,3 +277,41 @@ func (scs *Service) CheckCanInviteToSharedChannel(channelId string) error { } return nil } + +// updateMembershipSyncCursor updates the LastMembersSyncAt value for the shared channel remote +// This provides centralized and consistent cursor management +func (scs *Service) updateMembershipSyncCursor(channelID string, remoteID string, newTimestamp int64) error { + // Get the remote record + scr, err := scs.server.GetStore().SharedChannel().GetRemoteByIds(channelID, remoteID) + if err != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Failed to get shared channel remote for cursor update", + mlog.String("channel_id", channelID), + mlog.String("remote_id", remoteID), + mlog.Int("timestamp", int(newTimestamp)), + mlog.Err(err), + ) + return fmt.Errorf("failed to get shared channel remote for cursor update: %w", err) + } + + if scr == nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Shared channel remote not found for cursor update", + mlog.String("channel_id", channelID), + mlog.String("remote_id", remoteID), + ) + return fmt.Errorf("shared channel remote not found for channel %s and remote %s", channelID, remoteID) + } + + // Update the cursor - the store will handle ensuring it only moves forward + err = scs.server.GetStore().SharedChannel().UpdateRemoteMembershipCursor(scr.Id, newTimestamp) + if err != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Failed to update membership cursor", + mlog.String("channel_id", channelID), + mlog.String("remote_id", remoteID), + mlog.String("remote_record_id", scr.Id), + mlog.Int("timestamp", int(newTimestamp)), + mlog.Err(err), + ) + } + + return err +} diff --git a/server/platform/services/sharedchannel/sync_recv.go b/server/platform/services/sharedchannel/sync_recv.go index 2386919b88a..36d943cc45e 100644 --- a/server/platform/services/sharedchannel/sync_recv.go +++ b/server/platform/services/sharedchannel/sync_recv.go @@ -24,10 +24,9 @@ var ( ) func (scs *Service) onReceiveSyncMessage(msg model.RemoteClusterMsg, rc *model.RemoteCluster, response *remotecluster.Response) error { - if msg.Topic != TopicSync && msg.Topic != TopicGlobalUserSync { - return fmt.Errorf("wrong topic, expected `%s` or `%s`, got `%s`", TopicSync, TopicGlobalUserSync, msg.Topic) + if msg.Topic != TopicSync && msg.Topic != TopicChannelMembership && msg.Topic != TopicGlobalUserSync { + return fmt.Errorf("wrong topic, expected sync-related topic, got `%s`", msg.Topic) } - if len(msg.Payload) == 0 { return errors.New("empty sync message") } @@ -89,6 +88,15 @@ func (scs *Service) processSyncMessage(c request.CTX, syncMsg *model.SyncMsg, rc ReactionErrors: make([]string, 0), } + // Check if feature flag is enabled for membership changes + membershipSyncEnabled := scs.server.Config().FeatureFlags.EnableSharedChannelsMemberSync + hasMembershipChanges := len(syncMsg.MembershipChanges) > 0 + + // If this message only contains membership changes and feature is disabled, skip it + if hasMembershipChanges && !membershipSyncEnabled && len(syncMsg.Users) == 0 && len(syncMsg.Posts) == 0 && len(syncMsg.Reactions) == 0 { + return nil + } + scs.server.Log().Log(mlog.LvlSharedChannelServiceDebug, "Sync msg received", mlog.String("remote", rc.Name), mlog.String("channel_id", syncMsg.ChannelId), @@ -96,6 +104,7 @@ func (scs *Service) processSyncMessage(c request.CTX, syncMsg *model.SyncMsg, rc mlog.Int("post_count", len(syncMsg.Posts)), mlog.Int("reaction_count", len(syncMsg.Reactions)), mlog.Int("status_count", len(syncMsg.Statuses)), + mlog.Int("membership_change_count", len(syncMsg.MembershipChanges)), ) // Check if this is a global user sync message (no channel ID and only users) @@ -229,6 +238,19 @@ func (scs *Service) processSyncMessage(c request.CTX, syncMsg *model.SyncMsg, rc scs.app.SaveAndBroadcastStatus(status) } + // Process membership changes after users have been synced + if hasMembershipChanges && membershipSyncEnabled { + if err := scs.onReceiveMembershipChanges(syncMsg, rc, response); err != nil { + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Error processing membership changes", + mlog.String("remote", rc.Name), + mlog.String("channel_id", syncMsg.ChannelId), + mlog.Int("change_count", len(syncMsg.MembershipChanges)), + mlog.Err(err), + ) + // Don't fail the entire sync if membership changes fail + } + } + response.SetPayload(syncResp) return nil diff --git a/server/platform/services/sharedchannel/sync_send.go b/server/platform/services/sharedchannel/sync_send.go index 9aa22e26597..6e856224eaf 100644 --- a/server/platform/services/sharedchannel/sync_send.go +++ b/server/platform/services/sharedchannel/sync_send.go @@ -37,8 +37,17 @@ func newSyncTask(channelID, userID string, remoteID string, existingMsg, retryMs retryID = retryMsg.Id } + // Generate a unique task ID + taskID := channelID + userID + remoteID + retryID // combination of ids to avoid duplicates + + // For batch tasks, add a batch identifier to make the ID unique + if existingMsg != nil && len(existingMsg.MembershipChanges) > 1 { + batchID := model.NewId()[:8] // Use a short unique ID for the batch + taskID = channelID + "batch" + batchID + remoteID + retryID + } + return syncTask{ - id: channelID + userID + remoteID + retryID, // combination of ids to avoid duplicates + id: taskID, channelID: channelID, userID: userID, remoteID: remoteID, // empty means update all remote clusters @@ -235,6 +244,7 @@ func (scs *Service) ForceSyncForRemote(rc *model.RemoteCluster) { // addTask adds or re-adds a task to the queue. func (scs *Service) addTask(task syncTask) { task.AddedAt = time.Now() + scs.mux.Lock() if originalTask, ok := scs.tasks[task.id]; ok { // if the task was already scheduled, we only update the @@ -365,6 +375,16 @@ func (scs *Service) removeOldestTask() (syncTask, bool, time.Duration) { // processTask updates one or more remote clusters with any new channel content. func (scs *Service) processTask(task syncTask) error { + // Check if this is a membership change task + if task.existingMsg != nil && len(task.existingMsg.MembershipChanges) > 0 { + // Check if feature flag is enabled + if !scs.server.Config().FeatureFlags.EnableSharedChannelsMemberSync { + return nil + } + scs.processMembershipChange(task.existingMsg) + return nil + } + // map is used to ensure remotes don't get sync'd twice, such as when // they have the autoinvited flag and have explicitly subscribed to a channel. remotesMap := make(map[string]*model.RemoteCluster) diff --git a/server/platform/services/sharedchannel/sync_send_remote.go b/server/platform/services/sharedchannel/sync_send_remote.go index e72221e65af..f8c9ebd865f 100644 --- a/server/platform/services/sharedchannel/sync_send_remote.go +++ b/server/platform/services/sharedchannel/sync_send_remote.go @@ -119,6 +119,7 @@ func (scs *Service) syncForRemote(task syncTask, rc *model.RemoteCluster) error RemoteId: rc.RemoteId, LastPostCreateAt: model.GetMillis(), LastPostUpdateAt: model.GetMillis(), + LastMembersSyncAt: 0, } if scr, err = scs.server.GetStore().SharedChannel().SaveRemote(scr); err != nil { return fmt.Errorf("cannot auto-create shared channel remote (channel_id=%s, remote_id=%s): %w", task.channelID, rc.RemoteId, err) diff --git a/server/public/model/channel.go b/server/public/model/channel.go index 1d7309f5ba5..919fd7a53fd 100644 --- a/server/public/model/channel.go +++ b/server/public/model/channel.go @@ -507,3 +507,15 @@ type GroupMessageConversionRequestBody struct { Name string `json:"name"` DisplayName string `json:"display_name"` } + +// ChannelMembersGetOptions provides parameters for getting channel members +type ChannelMembersGetOptions struct { + // ChannelID specifies which channel to get members for + ChannelID string + // Offset for pagination + Offset int + // Limit for pagination (maximum number of results to return) + Limit int + // UpdatedAfter filters members updated after the given timestamp (cursor-based pagination) + UpdatedAfter int64 +} diff --git a/server/public/model/config.go b/server/public/model/config.go index 3a234d0e30c..cdd944a5402 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -49,7 +49,8 @@ const ( PasswordMaximumLength = 72 PasswordMinimumLength = 5 - ServiceGitlab = "gitlab" + ServiceGitlab = "gitlab" + ServiceGoogle = "google" ServiceOffice365 = "office365" ServiceOpenid = "openid" @@ -285,7 +286,8 @@ const ( LocalModeSocketPath = "/var/tmp/mattermost_local.socket" - ConnectedWorkspacesSettingsDefaultMaxPostsPerSync = 50 // a bit more than 4 typical screenfulls of posts + ConnectedWorkspacesSettingsDefaultMaxPostsPerSync = 50 // a bit more than 4 typical screenfulls of posts + ConnectedWorkspacesSettingsDefaultMemberSyncBatchSize = 20 // optimal batch size for syncing channel members // These storage classes are the valid values for the x-amz-storage-class header. More documentation here https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObject.html#AmazonS3-PutObject-request-header-StorageClass StorageClassStandard = "STANDARD" @@ -3462,6 +3464,7 @@ type ConnectedWorkspacesSettings struct { SyncUsersOnConnectionOpen *bool GlobalUserSyncBatchSize *int MaxPostsPerSync *int + MemberSyncBatchSize *int // Maximum number of members to process in a single batch during shared channel synchronization } func (c *ConnectedWorkspacesSettings) SetDefaults(isUpdate bool, e ExperimentalSettings) { @@ -3496,6 +3499,10 @@ func (c *ConnectedWorkspacesSettings) SetDefaults(isUpdate bool, e ExperimentalS if c.MaxPostsPerSync == nil { c.MaxPostsPerSync = NewPointer(ConnectedWorkspacesSettingsDefaultMaxPostsPerSync) } + + if c.MemberSyncBatchSize == nil { + c.MemberSyncBatchSize = NewPointer(ConnectedWorkspacesSettingsDefaultMemberSyncBatchSize) + } } type GlobalRelayMessageExportSettings struct { diff --git a/server/public/model/feature_flags.go b/server/public/model/feature_flags.go index 6f826e767fb..5f67c16d95e 100644 --- a/server/public/model/feature_flags.go +++ b/server/public/model/feature_flags.go @@ -25,6 +25,9 @@ type FeatureFlags struct { // Enable plugins in shared channels. EnableSharedChannelsPlugins bool + // Enable synchronization of channel members in shared channels + EnableSharedChannelsMemberSync bool + // Enable syncing all users for remote clusters in shared channels EnableSyncAllUsersForRemoteCluster bool @@ -72,6 +75,7 @@ func (f *FeatureFlags) SetDefaults() { f.TestBoolFeature = false f.EnableRemoteClusterService = false f.EnableSharedChannelsDMs = false + f.EnableSharedChannelsMemberSync = false f.EnableSyncAllUsersForRemoteCluster = false f.EnableSharedChannelsPlugins = true f.AppsEnabled = false diff --git a/server/public/model/shared_channel.go b/server/public/model/shared_channel.go index 9aec7197203..e09b3746d19 100644 --- a/server/public/model/shared_channel.go +++ b/server/public/model/shared_channel.go @@ -118,6 +118,7 @@ type SharedChannelRemote struct { LastPostUpdateID string `json:"last_post_id"` LastPostCreateAt int64 `json:"last_post_create_at"` LastPostCreateID string `json:"last_post_create_id"` + LastMembersSyncAt int64 `json:"last_members_sync_at"` } func (sc *SharedChannelRemote) IsValid() *AppError { @@ -169,12 +170,13 @@ type SharedChannelRemoteStatus struct { // SharedChannelUser stores a lastSyncAt timestamp on behalf of a remote cluster for // each user that has been synchronized. type SharedChannelUser struct { - Id string `json:"id"` - UserId string `json:"user_id"` - ChannelId string `json:"channel_id"` - RemoteId string `json:"remote_id"` - CreateAt int64 `json:"create_at"` - LastSyncAt int64 `json:"last_sync_at"` + Id string `json:"id"` + UserId string `json:"user_id"` + ChannelId string `json:"channel_id"` + RemoteId string `json:"remote_id"` + CreateAt int64 `json:"create_at"` + LastSyncAt int64 `json:"last_sync_at"` + LastMembershipSyncAt int64 `json:"last_membership_sync_at"` } func (scu *SharedChannelUser) PreSave() { @@ -270,15 +272,25 @@ type SharedChannelRemoteFilterOpts struct { IncludeDeleted bool } +// MembershipChangeMsg represents a change in channel membership +type MembershipChangeMsg struct { + ChannelId string `json:"channel_id"` + UserId string `json:"user_id"` + IsAdd bool `json:"is_add"` + RemoteId string `json:"remote_id"` + ChangeTime int64 `json:"change_time"` +} + // SyncMsg represents a change in content (post add/edit/delete, reaction add/remove, users). // It is sent to remote clusters as the payload of a `RemoteClusterMsg`. type SyncMsg struct { - Id string `json:"id"` - ChannelId string `json:"channel_id"` - Users map[string]*User `json:"users,omitempty"` - Posts []*Post `json:"posts,omitempty"` - Reactions []*Reaction `json:"reactions,omitempty"` - Statuses []*Status `json:"statuses,omitempty"` + Id string `json:"id"` + ChannelId string `json:"channel_id"` + Users map[string]*User `json:"users,omitempty"` + Posts []*Post `json:"posts,omitempty"` + Reactions []*Reaction `json:"reactions,omitempty"` + Statuses []*Status `json:"statuses,omitempty"` + MembershipChanges []*MembershipChangeMsg `json:"membership_changes,omitempty"` } func NewSyncMsg(channelID string) *SyncMsg {