feat: implement TTL cache for message deduplication and remove debug logging

- Replace manual map-based deduplication with jellydator/ttlcache/v3
- Add automatic cache eviction with 30-second TTL to prevent memory bloat
- Implement proper cache lifecycle management (start/stop)
- Remove emoji debug logs from bridge system and XMPP client
- Clean up verbose logging while maintaining essential error handling
- Update bridge interface method names for consistency

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

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Felipe M 2025-08-05 12:19:44 +02:00
parent 7b56cb34c6
commit eb852662f7
No known key found for this signature in database
GPG key ID: 52E5D65FCF99808A
9 changed files with 163 additions and 105 deletions

View file

@ -14,15 +14,15 @@ import (
// BridgeManager manages multiple bridge instances
type BridgeManager struct {
bridges map[string]model.Bridge
mu sync.RWMutex
logger logger.Logger
api plugin.API
remoteID string
messageBus model.MessageBus
routingCtx context.Context
bridges map[string]model.Bridge
mu sync.RWMutex
logger logger.Logger
api plugin.API
remoteID string
messageBus model.MessageBus
routingCtx context.Context
routingCancel context.CancelFunc
routingWg sync.WaitGroup
routingWg sync.WaitGroup
}
// NewBridgeManager creates a new bridge manager
@ -163,6 +163,20 @@ func (m *BridgeManager) ListBridges() []string {
return bridges
}
// Start starts the bridge manager and message routing system
func (m *BridgeManager) Start() error {
m.logger.LogInfo("Starting bridge manager")
// Start the message routing system
if err := m.StartMessageRouting(); err != nil {
m.logger.LogError("Failed to start message routing", "error", err)
return fmt.Errorf("failed to start message routing: %w", err)
}
m.logger.LogInfo("Bridge manager started successfully")
return nil
}
// HasBridge checks if a bridge with the given name is registered
func (m *BridgeManager) HasBridge(name string) bool {
m.mu.RLock()
@ -187,6 +201,11 @@ func (m *BridgeManager) Shutdown() error {
m.logger.LogInfo("Shutting down bridge manager", "bridge_count", len(m.bridges))
// Stop message routing first
if err := m.StopMessageRouting(); err != nil {
m.logger.LogError("Failed to stop message routing during shutdown", "error", err)
}
var errors []error
for name, bridge := range m.bridges {
if bridge.IsConnected() {
@ -260,7 +279,7 @@ func (m *BridgeManager) CreateChannelMapping(req model.CreateChannelMappingReque
}
// NEW: Check if room already mapped to another channel
existingChannelID, err := bridge.GetRoomMapping(req.BridgeRoomID)
existingChannelID, err := bridge.GetChannelMapping(req.BridgeRoomID)
if err != nil {
m.logger.LogError("Failed to check room mapping", "bridge_room_id", req.BridgeRoomID, "error", err)
return fmt.Errorf("failed to check room mapping: %w", err)
@ -274,7 +293,7 @@ func (m *BridgeManager) CreateChannelMapping(req model.CreateChannelMappingReque
}
// NEW: Check if room exists on target bridge
roomExists, err := bridge.RoomExists(req.BridgeRoomID)
roomExists, err := bridge.ChannelMappingExists(req.BridgeRoomID)
if err != nil {
m.logger.LogError("Failed to check room existence", "bridge_room_id", req.BridgeRoomID, "error", err)
return fmt.Errorf("failed to check room existence: %w", err)
@ -422,16 +441,16 @@ func (m *BridgeManager) unshareChannel(channelID string) error {
// startBridgeMessageHandler starts message handling for a specific bridge
func (m *BridgeManager) startBridgeMessageHandler(bridgeName string, bridge model.Bridge) {
m.logger.LogDebug("Starting message handler for bridge", "bridge", bridgeName)
// Subscribe to message bus
messageChannel := m.messageBus.Subscribe(bridgeName)
// Start message routing goroutine
m.routingWg.Add(1)
go func() {
defer m.routingWg.Done()
defer m.logger.LogDebug("Message handler stopped for bridge", "bridge", bridgeName)
for {
select {
case msg, ok := <-messageChannel:
@ -439,27 +458,27 @@ func (m *BridgeManager) startBridgeMessageHandler(bridgeName string, bridge mode
m.logger.LogDebug("Message channel closed for bridge", "bridge", bridgeName)
return
}
if err := m.handleBridgeMessage(bridgeName, bridge, msg); err != nil {
m.logger.LogError("Failed to handle message for bridge",
m.logger.LogError("Failed to handle message for bridge",
"bridge", bridgeName,
"source_bridge", msg.SourceBridge,
"error", err)
}
case <-m.routingCtx.Done():
m.logger.LogDebug("Context cancelled, stopping message handler", "bridge", bridgeName)
return
}
}
}()
// Listen to bridge's outgoing messages
m.routingWg.Add(1)
go func() {
defer m.routingWg.Done()
defer m.logger.LogDebug("Bridge message listener stopped", "bridge", bridgeName)
bridgeMessageChannel := bridge.GetMessageChannel()
for {
select {
@ -468,14 +487,14 @@ func (m *BridgeManager) startBridgeMessageHandler(bridgeName string, bridge mode
m.logger.LogDebug("Bridge message channel closed", "bridge", bridgeName)
return
}
if err := m.messageBus.Publish(msg); err != nil {
m.logger.LogError("Failed to publish message from bridge",
m.logger.LogError("Failed to publish message from bridge",
"bridge", bridgeName,
"direction", msg.Direction,
"error", err)
}
case <-m.routingCtx.Done():
m.logger.LogDebug("Context cancelled, stopping bridge listener", "bridge", bridgeName)
return
@ -486,26 +505,26 @@ func (m *BridgeManager) startBridgeMessageHandler(bridgeName string, bridge mode
// handleBridgeMessage processes an incoming message for a specific bridge
func (m *BridgeManager) handleBridgeMessage(bridgeName string, bridge model.Bridge, msg *model.DirectionalMessage) error {
m.logger.LogDebug("Handling message for bridge",
m.logger.LogDebug("Handling message for bridge",
"target_bridge", bridgeName,
"source_bridge", msg.SourceBridge,
"direction", msg.Direction,
"channel_id", msg.SourceChannelID)
// Get the bridge's message handler
handler := bridge.GetMessageHandler()
if handler == nil {
return fmt.Errorf("bridge %s does not have a message handler", bridgeName)
}
// Check if the handler can process this message
if !handler.CanHandleMessage(msg.BridgeMessage) {
m.logger.LogDebug("Bridge cannot handle message",
m.logger.LogDebug("Bridge cannot handle message",
"bridge", bridgeName,
"message_type", msg.MessageType)
return nil // Not an error, just skip
}
// Process the message
return handler.ProcessMessage(msg)
}
@ -519,15 +538,15 @@ func (m *BridgeManager) StartMessageRouting() error {
// StopMessageRouting stops the message bus and routing system
func (m *BridgeManager) StopMessageRouting() error {
m.logger.LogInfo("Stopping message routing system")
// Cancel routing context
if m.routingCancel != nil {
m.routingCancel()
}
// Wait for all routing goroutines to finish
m.routingWg.Wait()
// Stop the message bus
return m.messageBus.Stop()
}

View file

@ -25,6 +25,7 @@ type mattermostBridge struct {
api plugin.API
kvstore kvstore.KVStore
userManager pluginModel.BridgeUserManager
botUserID string // Bot user ID for posting messages
// Message handling
messageHandler *mattermostMessageHandler
@ -46,12 +47,13 @@ type mattermostBridge struct {
}
// NewBridge creates a new Mattermost bridge
func NewBridge(log logger.Logger, api plugin.API, kvstore kvstore.KVStore, cfg *config.Configuration) pluginModel.Bridge {
func NewBridge(log logger.Logger, api plugin.API, kvstore kvstore.KVStore, cfg *config.Configuration, botUserID string) pluginModel.Bridge {
ctx, cancel := context.WithCancel(context.Background())
b := &mattermostBridge{
logger: log,
api: api,
kvstore: kvstore,
botUserID: botUserID,
ctx: ctx,
cancel: cancel,
channelMappings: make(map[string]string),
@ -304,8 +306,8 @@ func (b *mattermostBridge) DeleteChannelMapping(channelID string) error {
return nil
}
// RoomExists checks if a Mattermost channel exists on the server
func (b *mattermostBridge) RoomExists(roomID string) (bool, error) {
// ChannelMappingExists checks if a Mattermost channel exists on the server
func (b *mattermostBridge) ChannelMappingExists(roomID string) (bool, error) {
if b.api == nil {
return false, fmt.Errorf("Mattermost API not initialized")
}
@ -354,6 +356,21 @@ func (b *mattermostBridge) GetRoomMapping(roomID string) (string, error) {
return channelID, nil
}
// GetChannelMappingForBridge retrieves the Mattermost channel ID for a given room ID from a specific bridge
func (b *mattermostBridge) GetChannelMappingForBridge(bridgeName, roomID string) (string, error) {
channelIDBytes, err := b.kvstore.Get(kvstore.BuildChannelMapKey(bridgeName, roomID))
if err != nil {
// No mapping found is not an error, just return empty string
b.logger.LogDebug("No channel mapping found for bridge room", "bridge_name", bridgeName, "room_id", roomID)
return "", nil
}
channelID := string(channelIDBytes)
b.logger.LogDebug("Found channel mapping for bridge room", "bridge_name", bridgeName, "room_id", roomID, "channel_id", channelID)
return channelID, nil
}
// GetUserManager returns the user manager for this bridge
func (b *mattermostBridge) GetUserManager() pluginModel.BridgeUserManager {
return b.userManager

View file

@ -62,8 +62,8 @@ func (h *mattermostMessageHandler) postMessageToMattermost(msg *pluginModel.Brid
return fmt.Errorf("Mattermost API not initialized")
}
// Get the Mattermost channel ID from the channel mapping
channelID, err := h.bridge.GetRoomMapping(msg.SourceChannelID)
// Get the Mattermost channel ID from the channel mapping using the source bridge name
channelID, err := h.bridge.GetChannelMappingForBridge(msg.SourceBridge, msg.SourceChannelID)
if err != nil {
return fmt.Errorf("failed to get channel mapping: %w", err)
}
@ -87,14 +87,15 @@ func (h *mattermostMessageHandler) postMessageToMattermost(msg *pluginModel.Brid
// Create the post
post := &mmModel.Post{
ChannelId: channelID,
UserId: h.bridge.botUserID,
Message: content,
Type: mmModel.PostTypeDefault,
Props: map[string]interface{}{
"from_bridge": msg.SourceBridge,
"bridge_user_id": msg.SourceUserID,
"bridge_user_name": msg.SourceUserName,
"bridge_message_id": msg.MessageID,
"bridge_timestamp": msg.Timestamp.Unix(),
"from_bridge": msg.SourceBridge,
"bridge_user_id": msg.SourceUserID,
"bridge_user_name": msg.SourceUserName,
"bridge_message_id": msg.MessageID,
"bridge_timestamp": msg.Timestamp.Unix(),
},
}
@ -204,4 +205,4 @@ func (r *mattermostUserResolver) GetDisplayName(externalUserID string) string {
}
return user.Id[:8] // Show first 8 chars of ID as fallback
}
}

View file

@ -502,8 +502,8 @@ func (b *xmppBridge) DeleteChannelMapping(channelID string) error {
return nil
}
// RoomExists checks if an XMPP room exists on the remote service
func (b *xmppBridge) RoomExists(roomID string) (bool, error) {
// ChannelMappingExists checks if an XMPP room exists on the remote service
func (b *xmppBridge) ChannelMappingExists(roomID string) (bool, error) {
if !b.connected.Load() {
return false, fmt.Errorf("not connected to XMPP server")
}
@ -547,6 +547,22 @@ func (b *xmppBridge) GetRoomMapping(roomID string) (string, error) {
return channelID, nil
}
// GetChannelMappingForBridge retrieves the Mattermost channel ID for a given room ID from a specific bridge
func (b *xmppBridge) GetChannelMappingForBridge(bridgeName, roomID string) (string, error) {
// Look up the channel ID using the bridge name and room ID as the key
channelIDBytes, err := b.kvstore.Get(kvstore.BuildChannelMapKey(bridgeName, roomID))
if err != nil {
// No mapping found is not an error, just return empty string
b.logger.LogDebug("No channel mapping found for bridge room", "bridge_name", bridgeName, "room_id", roomID)
return "", nil
}
channelID := string(channelIDBytes)
b.logger.LogDebug("Found channel mapping for bridge room", "bridge_name", bridgeName, "room_id", roomID, "channel_id", channelID)
return channelID, nil
}
// GetUserManager returns the user manager for this bridge
func (b *xmppBridge) GetUserManager() pluginModel.BridgeUserManager {
return b.userManager
@ -554,44 +570,30 @@ func (b *xmppBridge) GetUserManager() pluginModel.BridgeUserManager {
// startMessageAggregation starts the message aggregation goroutine
func (b *xmppBridge) startMessageAggregation() {
b.logger.LogDebug("Starting XMPP message aggregation")
clientChannel := b.bridgeClient.GetMessageChannel()
for {
select {
case <-b.ctx.Done():
b.logger.LogDebug("Stopping XMPP message aggregation")
return
default:
// Aggregate messages from bridge client if available
if b.bridgeClient != nil {
clientChannel := b.bridgeClient.GetMessageChannel()
select {
case msg, ok := <-clientChannel:
if !ok {
b.logger.LogDebug("Bridge client message channel closed")
continue
}
// Forward to our bridge's message channel
select {
case b.incomingMessages <- msg:
b.logger.LogDebug("Message forwarded from bridge client",
"source_channel", msg.SourceChannelID,
"user_id", msg.SourceUserID)
default:
b.logger.LogWarn("Bridge message channel full, dropping message",
"source_channel", msg.SourceChannelID,
"user_id", msg.SourceUserID)
}
case <-b.ctx.Done():
return
default:
// No messages available, continue with other potential sources
}
case msg, ok := <-clientChannel:
if !ok {
b.logger.LogDebug("Bridge client message channel closed")
return
}
// TODO: Add aggregation from user client channels when implemented
// This is where we would aggregate from multiple XMPP user connections
// Forward to our bridge's message channel
select {
case b.incomingMessages <- msg:
// Message forwarded successfully
case <-b.ctx.Done():
return
default:
b.logger.LogWarn("Bridge message channel full, dropping message",
"source_channel", msg.SourceChannelID,
"user_id", msg.SourceUserID)
}
}
}
}

View file

@ -73,6 +73,9 @@ func (r DeleteChannelMappingRequest) Validate() error {
}
type BridgeManager interface {
// Start starts the bridge manager and message routing system.
Start() error
// 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.
RegisterBridge(name string, bridge Bridge) error
@ -138,11 +141,11 @@ type Bridge interface {
// DeleteChannelMapping removes a mapping between a Mattermost channel ID and a bridge room ID.
DeleteChannelMapping(channelID string) error
// RoomExists checks if a room/channel exists on the remote service.
RoomExists(roomID string) (bool, error)
// ChannelMappingExists checks if a room/channel exists on the remote service.
ChannelMappingExists(roomID string) (bool, error)
// GetRoomMapping retrieves the Mattermost channel ID for a given room ID (reverse lookup).
GetRoomMapping(roomID string) (string, error)
// GetChannelMappingForBridge retrieves the Mattermost channel ID for a given room ID from a specific bridge.
GetChannelMappingForBridge(bridgeName, roomID string) (string, error)
// IsConnected checks if the bridge is connected to the remote service.
IsConnected() bool

View file

@ -14,7 +14,6 @@ import (
"github.com/mattermost/mattermost-plugin-bridge-xmpp/server/logger"
pluginModel "github.com/mattermost/mattermost-plugin-bridge-xmpp/server/model"
"github.com/mattermost/mattermost-plugin-bridge-xmpp/server/store/kvstore"
"github.com/mattermost/mattermost-plugin-bridge-xmpp/server/xmpp"
"github.com/mattermost/mattermost/server/public/model"
"github.com/mattermost/mattermost/server/public/plugin"
"github.com/mattermost/mattermost/server/public/pluginapi"
@ -35,9 +34,6 @@ type Plugin struct {
// commandClient is the client used to register and execute slash commands.
commandClient command.Command
// xmppClient is the client used to communicate with XMPP servers.
xmppClient *xmpp.Client
// logger is the main plugin logger
logger logger.Logger
@ -70,15 +66,11 @@ func (p *Plugin) OnActivate() error {
p.kvstore = kvstore.NewKVStore(p.client)
p.initXMPPClient()
// Load configuration directly
cfg := p.getConfiguration()
p.logger.LogDebug("Loaded configuration in OnActivate", "config", cfg)
// Register the plugin for shared channels
if err := p.registerForSharedChannels(); err != nil {
p.logger.LogError("Failed to register for shared channels", "error", err)
return fmt.Errorf("failed to register for shared channels: %w", err)
}
@ -87,12 +79,16 @@ func (p *Plugin) OnActivate() error {
// Initialize and register bridges with current configuration
if err := p.initBridges(*cfg); err != nil {
p.logger.LogError("Failed to initialize bridges", "error", err)
return fmt.Errorf("failed to initialize bridges: %w", err)
}
p.commandClient = command.NewCommandHandler(p.client, p.bridgeManager)
// Start the bridge manager (this starts message routing)
if err := p.bridgeManager.Start(); err != nil {
return fmt.Errorf("failed to start bridge manager: %w", err)
}
// Start all bridges
for _, bridgeName := range p.bridgeManager.ListBridges() {
if err := p.bridgeManager.StartBridge(bridgeName); err != nil {
@ -145,18 +141,6 @@ func (p *Plugin) ExecuteCommand(c *plugin.Context, args *model.CommandArgs) (*mo
return response, nil
}
func (p *Plugin) initXMPPClient() {
cfg := p.getConfiguration()
p.xmppClient = xmpp.NewClient(
cfg.XMPPServerURL,
cfg.XMPPUsername,
cfg.XMPPPassword,
cfg.GetXMPPResource(),
p.remoteID,
p.logger,
)
}
func (p *Plugin) initBridges(cfg config.Configuration) error {
// Create and register XMPP bridge
xmppBridge := xmppbridge.NewBridge(
@ -176,6 +160,7 @@ func (p *Plugin) initBridges(cfg config.Configuration) error {
p.API,
p.kvstore,
&cfg,
p.botUserID,
)
if err := p.bridgeManager.RegisterBridge("mattermost", mattermostBridge); err != nil {

View file

@ -8,6 +8,7 @@ import (
"fmt"
"time"
"github.com/jellydator/ttlcache/v3"
"github.com/mattermost/mattermost-plugin-bridge-xmpp/server/logger"
"github.com/mattermost/mattermost-plugin-bridge-xmpp/server/model"
"mellium.im/sasl"
@ -26,6 +27,9 @@ const (
// msgBufferSize is the buffer size for incoming message channels
msgBufferSize = 1000
// messageDedupeTTL is the TTL for message deduplication cache
messageDedupeTTL = 30 * time.Second
)
// Client represents an XMPP client for communicating with XMPP servers.
@ -51,6 +55,9 @@ type Client struct {
// Message handling for bridge integration
incomingMessages chan *model.DirectionalMessage
// Message deduplication cache to handle XMPP server duplicates
dedupeCache *ttlcache.Cache[string, time.Time]
}
// MessageRequest represents a request to send a message.
@ -99,6 +106,14 @@ type UserProfile struct {
func NewClient(serverURL, username, password, resource, remoteID string, logger logger.Logger) *Client {
ctx, cancel := context.WithCancel(context.Background())
// Create TTL cache for message deduplication
dedupeCache := ttlcache.New(
ttlcache.WithTTL[string, time.Time](messageDedupeTTL),
)
// Start automatic cleanup in background
go dedupeCache.Start()
client := &Client{
serverURL: serverURL,
username: username,
@ -110,6 +125,7 @@ func NewClient(serverURL, username, password, resource, remoteID string, logger
cancel: cancel,
sessionReady: make(chan struct{}),
incomingMessages: make(chan *model.DirectionalMessage, msgBufferSize),
dedupeCache: dedupeCache,
}
// Create MUC client and set up message handling
@ -139,6 +155,7 @@ func (c *Client) SetServerDomain(domain string) {
// Connect establishes connection to the XMPP server
func (c *Client) Connect() error {
if c.session != nil {
return nil // Already connected
}
@ -269,10 +286,11 @@ func (c *Client) Disconnect() error {
// Continue with cleanup even on timeout
}
// Stop the TTL cache cleanup goroutine
c.dedupeCache.Stop()
// Cancel the client context
if c.cancel != nil {
c.cancel()
}
c.cancel()
c.logger.LogInfo("XMPP client disconnected successfully")
return nil
@ -653,6 +671,17 @@ func (c *Client) handleIncomingMessage(msg stanza.Message, t xmlstream.TokenRead
return nil
}
// Deduplicate messages using message ID and TTL cache
if msg.ID != "" {
// Check if this message ID is already in the cache (indicates duplicate)
if c.dedupeCache.Has(msg.ID) {
return nil
}
// Record this message in the cache with TTL
c.dedupeCache.Set(msg.ID, time.Now(), ttlcache.DefaultTTL)
}
// Extract channel and user information from JIDs
channelID, err := c.extractChannelID(msg.From)
if err != nil {
@ -688,10 +717,7 @@ func (c *Client) handleIncomingMessage(msg stanza.Message, t xmlstream.TokenRead
// Send to message channel (non-blocking)
select {
case c.incomingMessages <- directionalMsg:
c.logger.LogDebug("Message queued for processing",
"channel_id", channelID,
"user_id", userID,
"content_length", len(msgWithBody.Body))
// Message queued successfully
default:
c.logger.LogWarn("Message channel full, dropping message",
"channel_id", channelID,