feat: implement comprehensive loop prevention and architecture improvements

- Add comprehensive loop prevention at source level for all bridges:
  - XMPP bridge: Skip messages from own XMPP connection user
  - Mattermost bridge: Skip messages from bot user and remote users
- Remove cache from getOrCreateRemoteUser method for simplified user management
- Improve XMPP client architecture with direct handler delegation:
  - Add SetMessageHandler and GetJID methods to XMPP client
  - Move protocol normalization methods to client level
  - Implement handleIncomingXMPPMessage in XMPP bridge for business logic
- Fix message direction handling in XMPP message handler
- Add remote user invitation to shared channels via InviteRemoteToChannel API
- Clean up unused code and improve code formatting

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Felipe M 2025-08-06 17:16:52 +02:00
parent 11a32afc53
commit d9c0215b93
No known key found for this signature in database
GPG key ID: 52E5D65FCF99808A
6 changed files with 63 additions and 44 deletions

View file

@ -278,7 +278,7 @@ func (m *BridgeManager) CreateChannelMapping(req model.CreateChannelMappingReque
return fmt.Errorf("bridge '%s' is not connected", req.BridgeName)
}
// NEW: Check if room already mapped to another channel
// Check if channel mapping already exists on the bridge
existingChannelID, err := bridge.GetChannelMapping(req.BridgeChannelID)
if err != nil {
m.logger.LogError("Failed to check channel mapping", "bridge_channel_id", req.BridgeChannelID, "error", err)

View file

@ -3,7 +3,6 @@ package mattermost
import (
"fmt"
"strings"
"sync"
"github.com/mattermost/mattermost-plugin-bridge-xmpp/server/logger"
pluginModel "github.com/mattermost/mattermost-plugin-bridge-xmpp/server/model"
@ -12,18 +11,15 @@ import (
// mattermostMessageHandler handles incoming messages for the Mattermost bridge
type mattermostMessageHandler struct {
bridge *mattermostBridge
logger logger.Logger
userCache map[string]string // Maps "bridgeType:remoteID:userID" -> Mattermost user ID
cacheMu sync.RWMutex // Protects userCache
bridge *mattermostBridge
logger logger.Logger
}
// newMessageHandler creates a new Mattermost message handler
func newMessageHandler(bridge *mattermostBridge) *mattermostMessageHandler {
return &mattermostMessageHandler{
bridge: bridge,
logger: bridge.logger,
userCache: make(map[string]string),
bridge: bridge,
logger: bridge.logger,
}
}
@ -91,6 +87,16 @@ func (h *mattermostMessageHandler) postMessageToMattermost(msg *pluginModel.Brid
return fmt.Errorf("failed to get or create remote user: %w", err)
}
if err := h.bridge.api.InviteRemoteToChannel(channelID, msg.SourceRemoteID, remoteUserID, true); err != nil {
h.logger.LogError("Failed to invite remote user to channel",
"channel_id", msg.SourceChannelID,
"remote_user_id", remoteUserID,
"source_bridge", msg.SourceBridge,
"source_remote_id", msg.SourceRemoteID,
"err", err.Error(),
)
}
// Create the post using the remote user (no need for bridge formatting since it's posted as the actual user)
post := &mmModel.Post{
ChannelId: channelID,
@ -128,26 +134,6 @@ func (h *mattermostMessageHandler) postMessageToMattermost(msg *pluginModel.Brid
// getOrCreateRemoteUser gets or creates a remote user for incoming bridge messages
func (h *mattermostMessageHandler) getOrCreateRemoteUser(msg *pluginModel.BridgeMessage) (string, error) {
// Create cache key: "bridgeType:remoteID:userID"
cacheKey := fmt.Sprintf("%s:%s:%s", msg.SourceBridge, msg.SourceRemoteID, msg.SourceUserID)
// Check cache first
h.cacheMu.RLock()
if userID, exists := h.userCache[cacheKey]; exists {
h.cacheMu.RUnlock()
return userID, nil
}
h.cacheMu.RUnlock()
// Lock for user creation
h.cacheMu.Lock()
defer h.cacheMu.Unlock()
// Double-check cache after acquiring lock
if userID, exists := h.userCache[cacheKey]; exists {
return userID, nil
}
// Generate username from source info
username := h.generateUsername(msg.SourceUserID, msg.SourceUserName, msg.SourceBridge)
@ -158,7 +144,6 @@ func (h *mattermostMessageHandler) getOrCreateRemoteUser(msg *pluginModel.Bridge
if existingUser, appErr := h.bridge.api.GetUserByUsername(username); appErr == nil && existingUser != nil {
// Check if this user has the correct RemoteId
if existingUser.RemoteId != nil && *existingUser.RemoteId == msg.SourceRemoteID {
h.userCache[cacheKey] = existingUser.Id
h.logger.LogDebug("Found existing remote user",
"user_id", existingUser.Id,
"username", username,
@ -172,7 +157,6 @@ func (h *mattermostMessageHandler) getOrCreateRemoteUser(msg *pluginModel.Bridge
if existingUser, appErr := h.bridge.api.GetUserByEmail(email); appErr == nil && existingUser != nil {
// Check if this user has the correct RemoteId
if existingUser.RemoteId != nil && *existingUser.RemoteId == msg.SourceRemoteID {
h.userCache[cacheKey] = existingUser.Id
h.logger.LogDebug("Found existing remote user by email",
"user_id", existingUser.Id,
"email", email,
@ -188,7 +172,7 @@ func (h *mattermostMessageHandler) getOrCreateRemoteUser(msg *pluginModel.Bridge
Email: email,
FirstName: msg.SourceUserName,
Password: mmModel.NewId(),
RemoteId: &msg.SourceRemoteID,
RemoteId: mmModel.NewPointer(msg.SourceRemoteID),
}
// Try to create the user
@ -203,9 +187,6 @@ func (h *mattermostMessageHandler) getOrCreateRemoteUser(msg *pluginModel.Bridge
return "", fmt.Errorf("failed to create remote user: %w", appErr)
}
// Cache the result
h.userCache[cacheKey] = createdUser.Id
h.logger.LogInfo("Created remote user",
"user_id", createdUser.Id,
"username", username,

View file

@ -32,8 +32,8 @@ type xmppBridge struct {
kvstore kvstore.KVStore
bridgeClient *xmppClient.Client // Main bridge XMPP client connection
userManager pluginModel.BridgeUserManager
bridgeID string // Bridge identifier used for registration
remoteID string // Remote ID for shared channels
bridgeID string // Bridge identifier used for registration
remoteID string // Remote ID for shared channels
// Message handling
messageHandler *xmppMessageHandler
@ -181,7 +181,6 @@ func (b *xmppBridge) Start() error {
// Start connection monitor
go b.connectionMonitor()
b.logger.LogInfo("Mattermost to XMPP bridge started successfully")
return nil
}
@ -574,7 +573,6 @@ func (b *xmppBridge) GetUserManager() pluginModel.BridgeUserManager {
return b.userManager
}
// GetMessageChannel returns the channel for incoming messages from XMPP
func (b *xmppBridge) GetMessageChannel() <-chan *pluginModel.DirectionalMessage {
return b.incomingMessages
@ -638,6 +636,14 @@ func (b *xmppBridge) handleIncomingXMPPMessage(msg stanza.Message, t xmlstream.T
userID, displayName := b.bridgeClient.ExtractUserInfo(msg.From)
// Skip messages from our own XMPP user to prevent loops
if userID == b.bridgeClient.GetJID().String() {
b.logger.LogDebug("Skipping message from our own XMPP user to prevent loop",
"our_jid", b.bridgeClient.GetJID().String(),
"source_user_id", userID)
return nil
}
// Create bridge message
bridgeMessage := &pluginModel.BridgeMessage{
SourceBridge: b.bridgeID,

View file

@ -37,7 +37,7 @@ func (h *xmppMessageHandler) ProcessMessage(msg *pluginModel.DirectionalMessage)
}
// For incoming messages to XMPP, we send them to XMPP rooms
if msg.Direction == pluginModel.DirectionIncoming {
if msg.Direction == pluginModel.DirectionOutgoing {
return h.sendMessageToXMPP(msg.BridgeMessage)
}