Skip to content

Commit 5635a36

Browse files
committed
Demonstrate msg.InternalID and MessageSentAck aren't necessary
Also make sure FindCanonicalMsgID respects the requested protocol.
1 parent c02ad66 commit 5635a36

5 files changed

Lines changed: 28 additions & 105 deletions

File tree

bridge/bridge.go

Lines changed: 1 addition & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"time"
1414

1515
"github.com/matterbridge-org/matterbridge/bridge/config"
16-
"github.com/rs/xid"
1716
"github.com/sirupsen/logrus"
1817
)
1918

@@ -24,7 +23,6 @@ type Bridger interface {
2423
Disconnect() error
2524
NewHttpRequest(method, uri string, body io.Reader) (*http.Request, error)
2625
NewHttpClient(proxy string) (*http.Client, error)
27-
AckSentMessage(internal xid.ID, external string, channel string)
2826
}
2927

3028
type Bridge struct {
@@ -46,17 +44,7 @@ type Bridge struct {
4644
type Config struct {
4745
*Bridge
4846

49-
Remote chan config.Message
50-
MessageSentAck chan MessageSent
51-
}
52-
53-
// MessageSent is an acknowledgement received from a remote
54-
// network that a message has been successfully sent, along
55-
// with a protocol-dependent unique ID.
56-
type MessageSent struct {
57-
DestBridge *Bridge
58-
InternalID xid.ID
59-
ExternalID config.MessageSentID
47+
Remote chan config.Message
6048
}
6149

6250
// Factory is the factory function to create a bridge
@@ -416,16 +404,3 @@ func (b *Bridge) addAttachmentProcess(msg *config.Message, filename string, id s
416404

417405
return nil
418406
}
419-
420-
func (b *Config) AckSentMessage(internal xid.ID, external string, channel string) {
421-
go func() {
422-
b.MessageSentAck <- MessageSent{
423-
DestBridge: b.Bridge,
424-
InternalID: internal,
425-
ExternalID: config.MessageSentID{
426-
ChannelID: channel,
427-
ID: external,
428-
},
429-
}
430-
}()
431-
}

bridge/config/config.go

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"time"
1313

1414
"github.com/fsnotify/fsnotify"
15-
"github.com/rs/xid"
1615
"github.com/sirupsen/logrus"
1716
"github.com/spf13/viper"
1817
)
@@ -41,20 +40,19 @@ type MessageSentID struct {
4140
}
4241

4342
type Message struct {
44-
Text string `json:"text"`
45-
Channel string `json:"channel"`
46-
Username string `json:"username"`
47-
UserID string `json:"userid"` // userid on the bridge
48-
Avatar string `json:"avatar"`
49-
Account string `json:"account"`
50-
Event string `json:"event"`
51-
Protocol string `json:"protocol"`
52-
Gateway string `json:"gateway"`
53-
ParentID string `json:"parent_id"`
54-
Timestamp time.Time `json:"timestamp"`
55-
ID string `json:"id"`
56-
InternalID xid.ID
57-
Extra map[string][]interface{}
43+
Text string `json:"text"`
44+
Channel string `json:"channel"`
45+
Username string `json:"username"`
46+
UserID string `json:"userid"` // userid on the bridge
47+
Avatar string `json:"avatar"`
48+
Account string `json:"account"`
49+
Event string `json:"event"`
50+
Protocol string `json:"protocol"`
51+
Gateway string `json:"gateway"`
52+
ParentID string `json:"parent_id"`
53+
Timestamp time.Time `json:"timestamp"`
54+
ID string `json:"id"`
55+
Extra map[string][]interface{}
5856
}
5957

6058
func (m Message) ParentNotFound() bool {

bridge/xmpp/xmpp.go

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,6 @@ type Bxmpp struct {
2626

2727
avatarAvailability map[string]bool
2828
avatarMap map[string]string
29-
30-
// Mapping of sent origin-id to internal matterbridge ID of source msg
31-
// An internal ID may correspond to several XMPP origin-id because
32-
// of split message (eg. multiple file upload)
33-
messageMap map[string]xid.ID
3429
}
3530

3631
func New(cfg *bridge.Config) bridge.Bridger {
@@ -39,7 +34,6 @@ func New(cfg *bridge.Config) bridge.Bridger {
3934
xmppMap: make(map[string]string),
4035
avatarAvailability: make(map[string]bool),
4136
avatarMap: make(map[string]string),
42-
messageMap: make(map[string]xid.ID),
4337
}
4438
}
4539

@@ -102,7 +96,6 @@ func (b *Bxmpp) Send(msg config.Message) (string, error) {
10296
b.Log.Debugf("=> Sending attachement message %#v", rmsg)
10397

10498
originId := xid.New().String()
105-
b.messageMap[originId] = msg.InternalID
10699

107100
_, err = b.xc.Send(xmpp.Chat{
108101
Type: "groupchat",
@@ -122,7 +115,6 @@ func (b *Bxmpp) Send(msg config.Message) (string, error) {
122115
}
123116

124117
originId := xid.New().String()
125-
b.messageMap[originId] = msg.InternalID
126118

127119
// Post normal message.
128120
b.Log.Debugf("=> Sending message %#v", msg)
@@ -136,7 +128,7 @@ func (b *Bxmpp) Send(msg config.Message) (string, error) {
136128
return "", err
137129
}
138130

139-
return "", nil
131+
return originId, nil
140132
}
141133

142134
func (b *Bxmpp) createXMPP() error {
@@ -354,7 +346,6 @@ func (b *Bxmpp) handleUploadFile(msg *config.Message) error {
354346
}
355347

356348
originId := xid.New().String()
357-
b.messageMap[originId] = msg.InternalID
358349

359350
if _, err := b.xc.Send(xmpp.Chat{
360351
Type: "groupchat",
@@ -367,7 +358,6 @@ func (b *Bxmpp) handleUploadFile(msg *config.Message) error {
367358

368359
originId = xid.New().String()
369360

370-
b.messageMap[originId] = msg.InternalID
371361
if fileInfo.URL != "" {
372362
if _, err := b.xc.SendOOB(xmpp.Chat{
373363
Type: "groupchat",
@@ -406,12 +396,6 @@ func (b *Bxmpp) parseChannel(remote string) string {
406396
func (b *Bxmpp) skipMessage(message xmpp.Chat) bool {
407397
// skip messages from ourselves
408398
if b.parseNick(message.Remote) == b.GetString("Nick") {
409-
// We received our own message, we will further ignore it,
410-
// but first send back the ID it was assigned.
411-
if message.OriginID != "" {
412-
internal := b.messageMap[message.OriginID]
413-
b.AckSentMessage(internal, message.OriginID, b.parseChannel(message.Remote))
414-
}
415399
return true
416400
}
417401

gateway/gateway.go

Lines changed: 9 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ func (gw *Gateway) FindCanonicalMsgID(protocol string, externalID string) *xid.I
6969
// TODO: should we check if the mapping exists here? or is this method
7070
// only used when we're 100% sure?
7171
externalIDs, _ := gw.Messages.Peek(internalID)
72-
for _, downstreamMsgObj := range externalIDs {
73-
if externalID == downstreamMsgObj.ID {
72+
for _, brMsgId := range externalIDs {
73+
if brMsgId.br.Protocol == protocol && brMsgId.ID == externalID {
7474
return &internalID
7575
}
7676
}
@@ -99,43 +99,9 @@ func (gw *Gateway) AddBridge(cfg *config.Bridge) error {
9999

100100
br.HttpClient = http_client
101101

102-
// The channel to receive message IDs is shared with the
103-
// bridges, but is not kept in the gateway.
104-
messageAck := make(chan bridge.MessageSent, 100)
105-
// Start listening for sent message acknowledgements
106-
go func() {
107-
for ack := range messageAck {
108-
gw.logger.Warnf("Message %s has been acked by %s as ID: %s", ack.InternalID.String(), ack.DestBridge.Protocol, ack.ExternalID.ID)
109-
// TODO 2026: this was a comment in the previous ID handling. Not
110-
// sure what to do about ID changing on edit????
111-
//
112-
// Only add the message ID if it doesn't already exist
113-
//
114-
// For some bridges we always add/update the message ID.
115-
// This is necessary as msgIDs will change if a bridge returns
116-
// a different ID in response to edits.
117-
118-
// brMsgIDs is always initialized in Router.handleReceive(). However,
119-
// we may still receive a message we don't know about. Maybe matterbridge
120-
// was restarted, or another client is connected on the same account sending
121-
// messages, or the remote server is melting down and dinosaurs are walking
122-
// the Earth...
123-
brMsgIDs, exists := gw.Messages.Get(ack.InternalID)
124-
125-
if !exists {
126-
gw.logger.Warnf("Unknown message %s has been acked by %s as ID: %s", ack.InternalID.String(), ack.DestBridge.Protocol, ack.ExternalID.ID)
127-
continue
128-
}
129-
130-
brMsgIDs = append(brMsgIDs, &BrMsgID{ack.DestBridge, ack.DestBridge.Protocol + " " + ack.ExternalID.ID, ack.ExternalID.ChannelID})
131-
gw.Messages.Add(ack.InternalID, brMsgIDs)
132-
}
133-
}()
134-
135102
brconfig := &bridge.Config{
136-
Remote: gw.Message,
137-
MessageSentAck: messageAck,
138-
Bridge: br,
103+
Remote: gw.Message,
104+
Bridge: br,
139105
}
140106
// add the actual bridger for this protocol to this bridge using the bridgeMap
141107
if _, ok := gw.Router.BridgeMap[br.Protocol]; !ok {
@@ -220,19 +186,19 @@ func (gw *Gateway) SendMessage(
220186
}
221187

222188
defer func(t time.Time) {
223-
gw.logger.Debugf("=> Send from %s (%s) to %s (%s) took %s", msg.Account, rmsg.Channel, dest.Account, channel.Name, time.Since(t))
189+
gw.logger.Debugf("=> Send from %s (%s) to %s (%s) took %s", msg.Account, rmsg.Channel, dest.Account, channel.Name, time.Since(t))
224190
}(time.Now())
225191

226192
mID, err := dest.Send(msg)
227193
if err != nil {
228-
return mID, err
194+
return mID, err
229195
}
230196

231197
// append the message ID (mID) from this bridge (dest) to our brMsgIDs slice
232198
if mID != "" {
233-
gw.logger.Debugf("mID %s: %s", dest.Account, mID)
234-
return mID, nil
235-
// brMsgIDs = append(brMsgIDs, &BrMsgID{dest, dest.Protocol + " " + mID, channel.ID})
199+
gw.logger.Debugf("mID %s: %s", dest.Account, mID)
200+
return mID, nil
201+
// brMsgIDs = append(brMsgIDs, &BrMsgID{dest, dest.Protocol + " " + mID, channel.ID})
236202
}
237203
return "", nil
238204
}

gateway/router.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ func (r *Router) handleReceive() {
141141
// We add an internal UUID which will allow destination protocols
142142
// to send back their own ID(s) corresponding to the message go the
143143
// gateway in an asynchronous manner (for replies/reactions).
144-
msg.InternalID = xid.New()
144+
InternalID := xid.New()
145145

146146
r.handleEventGetChannelMembers(&msg)
147147
r.handleEventFailure(&msg)
@@ -171,7 +171,7 @@ func (r *Router) handleReceive() {
171171
BrMsgIDs = append(BrMsgIDs, &BrMsgID{msgBridge, msg.ID, msg.Channel})
172172
}
173173
// Even if it might be empty, already initialize the mapping
174-
gw.Messages.Add(msg.InternalID, BrMsgIDs)
174+
gw.Messages.Add(InternalID, BrMsgIDs)
175175

176176
// scatter
177177
results := make(chan string)
@@ -192,9 +192,9 @@ func (r *Router) handleReceive() {
192192
// gather
193193
// we record the associated message IDs on all the other bridges
194194
for brID := range results {
195-
BrMsgIDs, _ := gw.Messages.Get(msg.InternalID)
195+
BrMsgIDs, _ := gw.Messages.Get(InternalID)
196196
BrMsgIDs = append(BrMsgIDs, &BrMsgID{msgBridge, brID, msg.Channel})
197-
gw.Messages.Add(msg.InternalID, BrMsgIDs)
197+
gw.Messages.Add(InternalID, BrMsgIDs)
198198
}
199199
}
200200
}

0 commit comments

Comments
 (0)