From 1f45197aa85fda47fe0555bb04908fe232898d1c Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Fri, 1 Aug 2025 19:10:40 +0200 Subject: [PATCH] feat: refactor channel mapping with structured parameters and shared channel integration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add ChannelMappingRequest and ChannelMappingDeleteRequest structs with validation - Update BridgeManager interface to accept structured parameters instead of individual strings - Implement proper user ID and team ID propagation to shared channels - Add shared channel creation/deletion integration with Mattermost API - Update command handlers to provide user and team context - Enhance logging with comprehensive parameter tracking 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- server/bridge/manager.go | 151 ++++++++++++++++++++++++++------------ server/command/command.go | 19 ++++- server/model/bridge.go | 60 ++++++++++++++- server/model/strings.go | 40 ++++++++++ server/plugin.go | 4 +- 5 files changed, 223 insertions(+), 51 deletions(-) create mode 100644 server/model/strings.go diff --git a/server/bridge/manager.go b/server/bridge/manager.go index 3c90d2c..c873377 100644 --- a/server/bridge/manager.go +++ b/server/bridge/manager.go @@ -6,24 +6,33 @@ import ( "github.com/mattermost/mattermost-plugin-bridge-xmpp/server/logger" "github.com/mattermost/mattermost-plugin-bridge-xmpp/server/model" + mmModel "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/plugin" ) // Manager manages multiple bridge instances type Manager struct { - bridges map[string]model.Bridge - mu sync.RWMutex - logger logger.Logger + bridges map[string]model.Bridge + mu sync.RWMutex + logger logger.Logger + api plugin.API + remoteID string } // NewManager creates a new bridge manager -func NewManager(logger logger.Logger) model.BridgeManager { +func NewManager(logger logger.Logger, api plugin.API, remoteID string) model.BridgeManager { if logger == nil { panic("logger cannot be nil") } + if api == nil { + panic("plugin API cannot be nil") + } return &Manager{ - bridges: make(map[string]model.Bridge), - logger: logger, + bridges: make(map[string]model.Bridge), + logger: logger, + api: api, + remoteID: remoteID, } } @@ -216,36 +225,30 @@ func (m *Manager) OnPluginConfigurationChange(config any) error { } // OnChannelMappingCreated handles the creation of a channel mapping by calling the appropriate bridge -func (m *Manager) OnChannelMappingCreated(channelID, bridgeName, bridgeRoomID string) error { - // Input validation - if channelID == "" { - return fmt.Errorf("channelID cannot be empty") - } - if bridgeName == "" { - return fmt.Errorf("bridgeName cannot be empty") - } - if bridgeRoomID == "" { - return fmt.Errorf("bridgeRoomID cannot be empty") +func (m *Manager) OnChannelMappingCreated(req model.ChannelMappingRequest) error { + // Validate request + if err := req.Validate(); err != nil { + return fmt.Errorf("invalid mapping request: %w", err) } - m.logger.LogDebug("Creating channel mapping", "channel_id", channelID, "bridge_name", bridgeName, "bridge_room_id", bridgeRoomID) + m.logger.LogDebug("Creating channel mapping", "channel_id", req.ChannelID, "bridge_name", req.BridgeName, "bridge_room_id", req.BridgeRoomID, "user_id", req.UserID, "team_id", req.TeamID) // Get the specific bridge - bridge, err := m.GetBridge(bridgeName) + bridge, err := m.GetBridge(req.BridgeName) if err != nil { - m.logger.LogError("Failed to get bridge", "bridge_name", bridgeName, "error", err) - return fmt.Errorf("failed to get bridge '%s': %w", bridgeName, err) + m.logger.LogError("Failed to get bridge", "bridge_name", req.BridgeName, "error", err) + return fmt.Errorf("failed to get bridge '%s': %w", req.BridgeName, err) } // Check if bridge is connected if !bridge.IsConnected() { - return fmt.Errorf("bridge '%s' is not connected", bridgeName) + return fmt.Errorf("bridge '%s' is not connected", req.BridgeName) } // Create the channel mapping on the receiving bridge - if err = bridge.CreateChannelMapping(channelID, bridgeRoomID); err != nil { - m.logger.LogError("Failed to create channel mapping", "channel_id", channelID, "bridge_name", bridgeName, "bridge_room_id", bridgeRoomID, "error", err) - return fmt.Errorf("failed to create channel mapping for bridge '%s': %w", bridgeName, err) + if err = bridge.CreateChannelMapping(req.ChannelID, req.BridgeRoomID); err != nil { + m.logger.LogError("Failed to create channel mapping", "channel_id", req.ChannelID, "bridge_name", req.BridgeName, "bridge_room_id", req.BridgeRoomID, "error", err) + return fmt.Errorf("failed to create channel mapping for bridge '%s': %w", req.BridgeName, err) } mattermostBridge, err := m.GetBridge("mattermost") @@ -255,43 +258,47 @@ func (m *Manager) OnChannelMappingCreated(channelID, bridgeName, bridgeRoomID st } // Create the channel mapping in the Mattermost bridge - if err = mattermostBridge.CreateChannelMapping(channelID, bridgeRoomID); err != nil { - m.logger.LogError("Failed to create channel mapping in Mattermost bridge", "channel_id", channelID, "bridge_name", bridgeName, "bridge_room_id", bridgeRoomID, "error", err) + if err = mattermostBridge.CreateChannelMapping(req.ChannelID, req.BridgeRoomID); err != nil { + m.logger.LogError("Failed to create channel mapping in Mattermost bridge", "channel_id", req.ChannelID, "bridge_name", req.BridgeName, "bridge_room_id", req.BridgeRoomID, "error", err) return fmt.Errorf("failed to create channel mapping in Mattermost bridge: %w", err) } - m.logger.LogInfo("Successfully created channel mapping", "channel_id", channelID, "bridge_name", bridgeName, "bridge_room_id", bridgeRoomID) + // Share the channel using Mattermost's shared channels API + if err = m.shareChannel(req); err != nil { + m.logger.LogError("Failed to share channel", "channel_id", req.ChannelID, "bridge_room_id", req.BridgeRoomID, "error", err) + // Don't fail the entire operation if sharing fails, but log the error + m.logger.LogWarn("Channel mapping created but sharing failed - channel may not sync properly") + } + + m.logger.LogInfo("Successfully created channel mapping", "channel_id", req.ChannelID, "bridge_name", req.BridgeName, "bridge_room_id", req.BridgeRoomID) return nil } // OnChannelMappingDeleted handles the deletion of a channel mapping by calling the appropriate bridges -func (m *Manager) OnChannelMappingDeleted(channelID, bridgeName string) error { - // Input validation - if channelID == "" { - return fmt.Errorf("channelID cannot be empty") - } - if bridgeName == "" { - return fmt.Errorf("bridgeName cannot be empty") +func (m *Manager) OnChannelMappingDeleted(req model.ChannelMappingDeleteRequest) error { + // Validate request + if err := req.Validate(); err != nil { + return fmt.Errorf("invalid delete request: %w", err) } - m.logger.LogDebug("Deleting channel mapping", "channel_id", channelID, "bridge_name", bridgeName) + m.logger.LogDebug("Deleting channel mapping", "channel_id", req.ChannelID, "bridge_name", req.BridgeName, "user_id", req.UserID, "team_id", req.TeamID) // Get the specific bridge - bridge, err := m.GetBridge(bridgeName) + bridge, err := m.GetBridge(req.BridgeName) if err != nil { - m.logger.LogError("Failed to get bridge", "bridge_name", bridgeName, "error", err) - return fmt.Errorf("failed to get bridge '%s': %w", bridgeName, err) + m.logger.LogError("Failed to get bridge", "bridge_name", req.BridgeName, "error", err) + return fmt.Errorf("failed to get bridge '%s': %w", req.BridgeName, err) } // Check if bridge is connected if !bridge.IsConnected() { - return fmt.Errorf("bridge '%s' is not connected", bridgeName) + return fmt.Errorf("bridge '%s' is not connected", req.BridgeName) } // Delete the channel mapping from the specific bridge - if err = bridge.DeleteChannelMapping(channelID); err != nil { - m.logger.LogError("Failed to delete channel mapping", "channel_id", channelID, "bridge_name", bridgeName, "error", err) - return fmt.Errorf("failed to delete channel mapping for bridge '%s': %w", bridgeName, err) + if err = bridge.DeleteChannelMapping(req.ChannelID); err != nil { + m.logger.LogError("Failed to delete channel mapping", "channel_id", req.ChannelID, "bridge_name", req.BridgeName, "error", err) + return fmt.Errorf("failed to delete channel mapping for bridge '%s': %w", req.BridgeName, err) } // Also delete from Mattermost bridge to clean up reverse mappings @@ -302,11 +309,65 @@ func (m *Manager) OnChannelMappingDeleted(channelID, bridgeName string) error { } // Delete the channel mapping from the Mattermost bridge - if err = mattermostBridge.DeleteChannelMapping(channelID); err != nil { - m.logger.LogError("Failed to delete channel mapping from Mattermost bridge", "channel_id", channelID, "bridge_name", bridgeName, "error", err) + if err = mattermostBridge.DeleteChannelMapping(req.ChannelID); err != nil { + m.logger.LogError("Failed to delete channel mapping from Mattermost bridge", "channel_id", req.ChannelID, "bridge_name", req.BridgeName, "error", err) return fmt.Errorf("failed to delete channel mapping from Mattermost bridge: %w", err) } - m.logger.LogInfo("Successfully deleted channel mapping", "channel_id", channelID, "bridge_name", bridgeName) + // Unshare the channel using Mattermost's shared channels API + if err = m.unshareChannel(req.ChannelID); err != nil { + m.logger.LogError("Failed to unshare channel", "channel_id", req.ChannelID, "error", err) + // Don't fail the entire operation if unsharing fails, but log the error + m.logger.LogWarn("Channel mapping deleted but unsharing failed - channel may still appear as shared") + } + + m.logger.LogInfo("Successfully deleted channel mapping", "channel_id", req.ChannelID, "bridge_name", req.BridgeName) + return nil +} + +// shareChannel creates a shared channel configuration using the Mattermost API +func (m *Manager) shareChannel(req model.ChannelMappingRequest) error { + if m.remoteID == "" { + return fmt.Errorf("remote ID not set - plugin not registered for shared channels") + } + + // Create SharedChannel configuration + sharedChannel := &mmModel.SharedChannel{ + ChannelId: req.ChannelID, + TeamId: req.TeamID, + Home: true, + ReadOnly: false, + ShareName: model.SanitizeShareName(fmt.Sprintf("bridge-%s", req.BridgeRoomID)), + ShareDisplayName: fmt.Sprintf("Bridge: %s", req.BridgeRoomID), + SharePurpose: fmt.Sprintf("Shared channel bridged to %s", req.BridgeRoomID), + ShareHeader: "test header", + CreatorId: req.UserID, + RemoteId: m.remoteID, + } + + // Share the channel + sharedChannel, err := m.api.ShareChannel(sharedChannel) + if err != nil { + return fmt.Errorf("failed to share channel via API: %w", err) + } + + m.logger.LogInfo("Successfully shared channel", "channel_id", req.ChannelID, "shared_channel_id", sharedChannel.ChannelId) + return nil +} + +// unshareChannel removes shared channel configuration using the Mattermost API +func (m *Manager) unshareChannel(channelID string) error { + // Unshare the channel + unshared, err := m.api.UnshareChannel(channelID) + if err != nil { + return fmt.Errorf("failed to unshare channel via API: %w", err) + } + + if !unshared { + m.logger.LogWarn("Channel was not shared or already unshared", "channel_id", channelID) + } else { + m.logger.LogInfo("Successfully unshared channel", "channel_id", channelID) + } + return nil } diff --git a/server/command/command.go b/server/command/command.go index 5c40f20..931287e 100644 --- a/server/command/command.go +++ b/server/command/command.go @@ -152,7 +152,15 @@ func (c *Handler) executeMapCommand(args *model.CommandArgs, fields []string) *m } // Create the mapping using BridgeManager - err = c.bridgeManager.OnChannelMappingCreated(channelID, "xmpp", roomJID) + mappingReq := pluginModel.ChannelMappingRequest{ + ChannelID: channelID, + BridgeName: "xmpp", + BridgeRoomID: roomJID, + UserID: args.UserId, + TeamID: args.TeamId, + } + + err = c.bridgeManager.OnChannelMappingCreated(mappingReq) if err != nil { return &model.CommandResponse{ ResponseType: model.CommandResponseTypeEphemeral, @@ -195,7 +203,14 @@ func (c *Handler) executeUnmapCommand(args *model.CommandArgs) *model.CommandRes } // Delete the mapping - err = c.bridgeManager.OnChannelMappingDeleted(channelID, "xmpp") + deleteReq := pluginModel.ChannelMappingDeleteRequest{ + ChannelID: channelID, + BridgeName: "xmpp", + UserID: args.UserId, + TeamID: args.TeamId, + } + + err = c.bridgeManager.OnChannelMappingDeleted(deleteReq) if err != nil { return &model.CommandResponse{ ResponseType: model.CommandResponseTypeEphemeral, diff --git a/server/model/bridge.go b/server/model/bridge.go index b7f5e6a..799188f 100644 --- a/server/model/bridge.go +++ b/server/model/bridge.go @@ -1,5 +1,7 @@ package model +import "fmt" + type BridgeID string type UserState int @@ -11,6 +13,60 @@ const ( UserStateOffline ) +// ChannelMappingRequest contains information needed to create a channel mapping +type ChannelMappingRequest struct { + ChannelID string // Mattermost channel ID + BridgeName string // Name of the bridge (e.g., "xmpp") + BridgeRoomID string // Remote room/channel ID (e.g., JID for XMPP) + UserID string // ID of user who triggered the mapping creation + TeamID string // Team ID where the channel belongs +} + +// Validate checks if all required fields are present and valid +func (r ChannelMappingRequest) Validate() error { + if r.ChannelID == "" { + return fmt.Errorf("channelID cannot be empty") + } + if r.BridgeName == "" { + return fmt.Errorf("bridgeName cannot be empty") + } + if r.BridgeRoomID == "" { + return fmt.Errorf("bridgeRoomID cannot be empty") + } + if r.UserID == "" { + return fmt.Errorf("userID cannot be empty") + } + if r.TeamID == "" { + return fmt.Errorf("teamID cannot be empty") + } + return nil +} + +// ChannelMappingDeleteRequest contains information needed to delete a channel mapping +type ChannelMappingDeleteRequest struct { + ChannelID string // Mattermost channel ID + BridgeName string // Name of the bridge (e.g., "xmpp") + UserID string // ID of user who triggered the mapping deletion + TeamID string // Team ID where the channel belongs +} + +// Validate checks if all required fields are present and valid +func (r ChannelMappingDeleteRequest) Validate() error { + if r.ChannelID == "" { + return fmt.Errorf("channelID cannot be empty") + } + if r.BridgeName == "" { + return fmt.Errorf("bridgeName cannot be empty") + } + if r.UserID == "" { + return fmt.Errorf("userID cannot be empty") + } + if r.TeamID == "" { + return fmt.Errorf("teamID cannot be empty") + } + return nil +} + type BridgeManager interface { // RegisterBridge registers a bridge with the given name. Returns an error if the name is empty, // the bridge is nil, or a bridge with the same name is already registered. @@ -52,10 +108,10 @@ type BridgeManager interface { OnPluginConfigurationChange(config any) error // OnChannelMappingCreated is called when a channel mapping is created. - OnChannelMappingCreated(channelID, bridgeName, bridgeRoomID string) error + OnChannelMappingCreated(req ChannelMappingRequest) error // OnChannelMappingDeleted is called when a channel mapping is deleted. - OnChannelMappingDeleted(channelID, bridgeName string) error + OnChannelMappingDeleted(req ChannelMappingDeleteRequest) error } type Bridge interface { diff --git a/server/model/strings.go b/server/model/strings.go new file mode 100644 index 0000000..1e559d9 --- /dev/null +++ b/server/model/strings.go @@ -0,0 +1,40 @@ +package model + +import "strings" + +// sanitizeShareName creates a valid ShareName matching the regex: ^[a-z0-9]+([a-z\-\_0-9]+|(__)?)[a-z0-9]*$ +func SanitizeShareName(name string) string { + // Convert to lowercase and replace spaces with hyphens + shareName := strings.ToLower(name) + shareName = strings.ReplaceAll(shareName, " ", "-") + + // Remove any characters that aren't lowercase letters, numbers, hyphens, or underscores + var validShareName strings.Builder + for _, r := range shareName { + if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') || r == '-' || r == '_' { + validShareName.WriteRune(r) + } + } + + result := validShareName.String() + if result == "" { + return "matrixbridge" // fallback if no valid characters + } + + // Ensure it starts with alphanumeric + for len(result) > 0 && (result[0] == '-' || result[0] == '_') { + result = result[1:] + } + + // Ensure it ends with alphanumeric + for len(result) > 0 && (result[len(result)-1] == '-' || result[len(result)-1] == '_') { + result = result[:len(result)-1] + } + + // Final fallback check + if result == "" { + return "matrixbridge" + } + + return result +} diff --git a/server/plugin.go b/server/plugin.go index ded24bb..760d200 100644 --- a/server/plugin.go +++ b/server/plugin.go @@ -83,7 +83,7 @@ func (p *Plugin) OnActivate() error { } // Initialize bridge manager - p.bridgeManager = bridge.NewManager(p.logger) + p.bridgeManager = bridge.NewManager(p.logger, p.API, p.remoteID) // Initialize and register bridges with current configuration if err := p.initBridges(*cfg); err != nil { @@ -202,7 +202,7 @@ func (p *Plugin) registerForSharedChannels() error { PluginID: manifest.Id, CreatorID: botUserID, AutoShareDMs: false, - AutoInvited: false, + AutoInvited: true, } remoteID, appErr := p.API.RegisterPluginForSharedChannels(opts)