mirror of
https://github.com/hanzoai/team.git
synced 2026-09-30 01:57:35 +00:00
* MM-68339: Add XML struct tags and multi-remote registration for shared channels plugin API (#36126) * Add XML struct tags and multi-remote registration for shared channels plugin API Phase 1: Add xml struct tags to model types used in SyncMsg (Post, User, Reaction, Status, PostAcknowledgement, FileInfo, SyncResponse, MembershipChangeMsg). Add custom MarshalXML/UnmarshalXML for SyncMsg (Users map, MentionTransforms map), StringMap, and StringInterface. Exclude Post.Metadata, PrevStatus, and server-internal FileInfo fields from XML. JSON serialization is unaffected. Phase 2: Lift the one-remote-per-plugin constraint so plugins can register multiple remotes with different SiteURLs. Add SiteURL field to RegisterPluginOpts (defaults to "plugin_<PluginID>" for backward compatibility). Add GetAllByPluginID and GetBySiteURL store methods. Rewrite registration to dedup by SiteURL instead of PluginID. Add UnregisterPluginRemoteForSharedChannels for single-remote removal with plugin ownership validation. Validate SiteURL is non-empty in RemoteCluster.IsValid. Simplify IsPlugin() to check PluginID only. (cherry picked from commita760aefcd0) * MM-68339: slugify RemoteCluster.Name in plugin registration (#36309) The new TestRegisterPluginForSharedChannels tests added in #36126 broke master CI because RegisterPluginForSharedChannels assigned opts.Displayname directly to RemoteCluster.Name, which IsValid validates against the slug regex ^[a-zA-Z0-9.\-_]+$. Display names with spaces (e.g. "legacy plugin") fail validation. The tests didn't run in the PR's final CI shard and the issue surfaced post-merge. Add CleanRemoteName to the public model, mirroring CleanTeamName and CleanUsername: lowercase, replace spaces and other disallowed characters with hyphens, trim, truncate to RemoteNameMaxLength, fall back to NewId when the result is empty. Use it in RegisterPluginForSharedChannels so Name is always slug-valid while DisplayName keeps the human-readable label. This also lets real plugins register with display names containing spaces. (cherry picked from commit078f6b31b5) * MM-68622: start inter-cluster services before plugin activation (#36366) * MM-68622: start inter-cluster services before plugin activation Move startInterClusterServices from the end of Server.Start() to the beginning, before Channels().Start() initializes plugins. This lets plugins call shared channels APIs (ShareChannel, InviteRemoteToChannel, UninviteRemoteFromChannel, UnshareChannel, UpdateSharedChannel, CheckCanInviteToSharedChannel) during OnActivate instead of failing with "Shared Channels Service is disabled". Side-effect analysis: * Plugin API gating: getSharedChannelsService in channels/app/shared_channel.go:32 only requires the service to be non-nil. The plugin-facing wrappers all pass ensureIsActive=false, so Active() is bypassed. Once SetSharedChannelService runs, calls succeed on both leader and follower nodes. This is the fix path. * Multi-node leader timing: the enterprise cluster impl in enterprise/cluster/cluster.go:70 initializes currentLeader="", so IsLeader() returns false before StartInterNodeCommunication runs. The immediate onClusterLeaderChange in scs.Start at platform/services/sharedchannel/service.go:151 therefore takes the pause path, which is a no-op since the service was never active. When memberlist.Create fires NotifyJoin for the local node, addPotentialLeader runs and InvokeClusterLeaderChangedListeners drives the registered listener to resume() the sync loop on the elected leader. End state matches the prior ordering. * Single-node: IsLeader() returns true unconditionally per channels/app/platform/cluster.go:33, so SharedChannelSyncHandler is active during plugin OnActivate. Events emitted by plugins during activation (posts to shared channels, DM creation) now flow through sync where they were previously dropped. This is intended correctness, not a regression. * Transport and handlers: api4 remote-cluster routes are registered before Server.Start, so HTTP handlers exist when rcs.Start runs early. rcs and scs do not send cluster-broadcast messages during Start; they only register topic listeners on the rcs transport, which is independent of cluster gossip. registerClusterHandlers ordering is unaffected. * Config: scs reads only ConnectedWorkspacesSettings and the License at construction, both stable from the initial config load. ReloadConfig at server.go:912 has no bearing on inter-cluster service init. Errors from startInterClusterServices remain logged and non-fatal, matching prior behavior. (cherry picked from commit4e967cb7ad) * MM-68536: Show actual remote names in system console channel list (#36298) * MM-68536: Show actual remote names in system console channel list The system console "Channels" list rendered SharedChannelIndicator without a remoteNames prop, so every shared channel showed the generic "Shared with trusted organizations" fallback even after PR 35908. Add a small connected wrapper that selects remote names via getRemoteNamesForChannel and dispatches fetchChannelRemotes on mount, mirroring the pattern used by the LHS sidebar. The shared_channel_remote_updated websocket event added in PR 35908 already refreshes the same Redux slice, so the system console list now stays in sync automatically. (cherry picked from commit15d7fbd8cb) * [MM-68697] Preserve sender file ID in plugin-relayed shared channel attachments (#36468) Set ReqFileId on the UploadSession created in ReceiveSharedChannelAttachmentSyncMsg so the attachment is persisted under the sender's file ID. Without this, the receiving server stored the bytes under a freshly generated ID while the synced post's FileIds still referenced the sender's ID, leaving the attachment invisible in the UI even though the file and FileInfo row existed on disk. Mirrors the existing cluster-to-cluster path in platform/services/sharedchannel/attachment.go. Also tightens TestPluginAPIReceiveSharedChannelAttachmentSyncMsg to pass a sender-side fi.Id and assert the saved FileInfo keeps it. The prior assertion only checked that some ID was assigned, which the buggy code also satisfied. (cherry picked from commita72b1db4c5) * MM-68705 - Order-tolerant Shared Channel plugin API's for receiving attachments (#36486) * MM-68705 - Make ReceiveSharedChannelAttachmentSyncMsg order tolerant Allow plugin remotes to invoke ReceiveSharedChannelAttachmentSyncMsg and ReceiveSharedChannelSyncMsg in either order without losing the post/file binding, and make repeat deliveries of the same file id idempotent. Two localised changes inside ReceiveSharedChannelAttachmentSyncMsg: - Idempotency check before CreateUploadSession. If a FileInfo with the sender's id already exists for the same channel and creator, return it instead of inserting a duplicate (which would violate the FileInfo PK and force the caller to retry indefinitely after any transient ack failure). A mismatched channel or creator on the same id is rejected. - Lazy bind to post after UploadData. When the matching post is already present (post-then-file ordering), CreatePost will have stripped the unmatched file id from Post.FileIds; AttachToPost plus Post.Overwrite restore the binding. When the post is not yet present (file-then-post ordering), the FileInfo is left unbound so the eventual post arrival's CreatePost path binds it. The post is fetched first because AttachToPost is a blind UPDATE that does not validate post existence, so calling it before the post exists would orphan the FileInfo. No other paths change. Cluster (non-plugin) shared-channel attachments, ReceiveSharedChannelProfileImageSyncMsg, and UI uploads are unaffected. New tests in shared_channel_test.go cover both orderings, repeated receive success and rejection on channel/creator mismatch, and the empty-PostId no-op. (cherry picked from commit5c58953e8e) * MM-68708 - Fix TestCreatePost shared DM/GM subtests when env pins feature flag (#36488) The two shared DM/GM subtests in TestCreatePost rely on FeatureFlags.EnableSharedChannelsDMs being false, but the config store reapplies MM_FEATUREFLAGS_* env overrides on every Set, so an UpdateConfig pin gets clobbered. Use t.Setenv to force the flag false for the duration of each subtest, and drop the parent's mainHelper.Parallel(t) so Go's "no Setenv under a parallel ancestor" rule is satisfied. Sibling subtests still parallelize via their own mainHelper.Parallel(t) calls. (cherry picked from commita98e01ebac) * MM-68838: Ping a restored plugin remote immediately on re-register (#36592) * MM-68838: ping restored plugin remote immediately on re-register RegisterPluginForSharedChannels' restore branch updated the row but did not call PingNow, leaving the restored remote offline until the next pingLoop tick (up to PingFreq, default 1 minute). The new-connection branch already calls PingNow; the restore branch now mirrors it so sync attempts immediately after a plugin restart no longer fail with "offline remote cluster". * MM-68838: gob-encode error returns in apiRPCServer.ReceiveSharedChannelAttachmentSyncMsg The apiRPCServer wrapper for ReceiveSharedChannelAttachmentSyncMsg assigned the hook's error return directly to the gob-encoded response struct. When the framework's App.ReceiveSharedChannelAttachmentSyncMsg returned an error wrapped with %w (*fmt.wrapError, an unexported type), gob refused to encode it and the RPC server broke the connection with "type not registered for interface: fmt.wrapError". Every subsequent plugin/server RPC call then returned the zero-value response struct, causing plugins that dereferenced the nil returns to crash. Apply the existing encodableError() helper so the returned error becomes a gob-safe ErrorString, matching every other apiRPCServer method in this file. (cherry picked from commitea7f488c64)
47 lines
1.8 KiB
Go
47 lines
1.8 KiB
Go
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
|
|
// See LICENSE.txt for license information.
|
|
|
|
package plugin
|
|
|
|
import (
|
|
"bytes"
|
|
"encoding/gob"
|
|
"errors"
|
|
"fmt"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
// TestReceiveSharedChannelAttachmentSyncMsgReturns_GobRoundTrip pins the fix
|
|
// for the gob-encoding bug in apiRPCServer.ReceiveSharedChannelAttachmentSyncMsg.
|
|
// The hook may return errors wrapped with fmt.Errorf("...%w", err), producing
|
|
// values of the unexported type *fmt.wrapError that gob refuses to encode.
|
|
// The RPC server must run the error through encodableError before assigning
|
|
// it to the returns struct. Without that, the RPC connection breaks and
|
|
// every subsequent plugin to server call returns zero values.
|
|
func TestReceiveSharedChannelAttachmentSyncMsgReturns_GobRoundTrip(t *testing.T) {
|
|
wrapped := fmt.Errorf("attachment sync failed: %w", errors.New("upstream boom"))
|
|
|
|
t.Run("raw wrapped error fails to gob-encode (reproduces the bug)", func(t *testing.T) {
|
|
returns := Z_ReceiveSharedChannelAttachmentSyncMsgReturns{B: wrapped}
|
|
|
|
var buf bytes.Buffer
|
|
err := gob.NewEncoder(&buf).Encode(&returns)
|
|
require.Error(t, err, "raw *fmt.wrapError must not be gob-encodable; if this assertion ever fails the bug guarded by encodableError no longer exists")
|
|
require.Contains(t, err.Error(), "fmt.wrapError")
|
|
})
|
|
|
|
t.Run("encodableError-wrapped error round-trips through gob", func(t *testing.T) {
|
|
returns := Z_ReceiveSharedChannelAttachmentSyncMsgReturns{B: encodableError(wrapped)}
|
|
|
|
var buf bytes.Buffer
|
|
require.NoError(t, gob.NewEncoder(&buf).Encode(&returns))
|
|
|
|
var decoded Z_ReceiveSharedChannelAttachmentSyncMsgReturns
|
|
require.NoError(t, gob.NewDecoder(&buf).Decode(&decoded))
|
|
require.Error(t, decoded.B)
|
|
require.Equal(t, wrapped.Error(), decoded.B.Error())
|
|
})
|
|
}
|