From a62d1bbcc9e1f1f9053c465b4248404a3329e3e8 Mon Sep 17 00:00:00 2001 From: John Brooks Date: Mon, 25 Sep 2017 13:01:07 -0700 Subject: [PATCH] Improve ChatChannel API for message acknowledgement ChatChannel didn't return the message ID for sent messages, which made using the returned ACKs impossible. The SendMessage method now returns the uin32 messageID. Also, SendMessage didn't support the TimeDelta field for messages, which is used for queued or resent messages. This is now available as SendMessageWithTime. And finally, the ChatMessageAck callback didn't indicate if mesasges were accepted or not, which is part of the protocol. That was added as a field, which is unfortunately a breaking API change, but I've made enough of those lately to not feel guilty about it. --- application/application.go | 2 +- channels/chatchannel.go | 38 ++++++++++++++++++++++-------------- channels/chatchannel_test.go | 4 ++-- examples/echobot/main.go | 2 +- utils/messagebuilder.go | 7 ++++--- utils/messagebuilder_test.go | 2 +- 6 files changed, 32 insertions(+), 23 deletions(-) diff --git a/application/application.go b/application/application.go index f891608..4331213 100644 --- a/application/application.go +++ b/application/application.go @@ -65,7 +65,7 @@ func (rai *RicochetApplicationInstance) ChatMessage(messageID uint32, when time. return true } -func (rai *RicochetApplicationInstance) ChatMessageAck(messageID uint32) { +func (rai *RicochetApplicationInstance) ChatMessageAck(messageID uint32, accepted bool) { rai.ChatMessageAckHandler(rai, messageID) } diff --git a/channels/chatchannel.go b/channels/chatchannel.go index c4e6e19..01ba01e 100644 --- a/channels/chatchannel.go +++ b/channels/chatchannel.go @@ -35,22 +35,34 @@ type ChatChannelHandler interface { // the message successfully, and false to NACK and refuse the message. ChatMessage(messageID uint32, when time.Time, message string) bool // ChatMessageAck is called when an acknowledgement of a sent message is received. - ChatMessageAck(messageID uint32) + ChatMessageAck(messageID uint32, accepted bool) } -// SendMessage sends a given message using this channe -func (cc *ChatChannel) SendMessage(message string) { +// SendMessage sends a given message using this channel, and returns the +// messageID, which will be used in ChatMessageAck when the peer acknowledges +// this message. +func (cc *ChatChannel) SendMessage(message string) uint32 { + return cc.SendMessageWithTime(message, time.Now()) +} + +// SendMessageWithTime is identical to SendMessage, but also sends the provided time.Time +// as a rough timestamp for when this message was originally sent. This should be used +// when retrying or sending queued messages. +func (cc *ChatChannel) SendMessageWithTime(message string, when time.Time) uint32 { + delta := time.Now().Sub(when) / time.Second messageBuilder := new(utils.MessageBuilder) - //TODO Implement Chat Number - data := messageBuilder.ChatMessage(message, cc.lastMessageID) + messageID := cc.lastMessageID cc.lastMessageID++ + data := messageBuilder.ChatMessage(message, messageID, int64(delta)) cc.channel.SendMessage(data) + return messageID } -// Acknowledge indicates the given messageID was received -func (cc *ChatChannel) Acknowledge(messageID uint32) { +// Acknowledge indicates that the given messageID was received, and whether +// it was accepted. +func (cc *ChatChannel) Acknowledge(messageID uint32, accepted bool) { messageBuilder := new(utils.MessageBuilder) - cc.channel.SendMessage(messageBuilder.AckChatMessage(messageID)) + cc.channel.SendMessage(messageBuilder.AckChatMessage(messageID, accepted)) } // Type returns the type string for this channel, e.g. "im.ricochet.chat". @@ -132,13 +144,9 @@ func (cc *ChatChannel) Packet(data []byte) { if err == nil { if res.GetChatMessage() != nil { ack := cc.Handler.ChatMessage(res.GetChatMessage().GetMessageId(), time.Now(), res.GetChatMessage().GetMessageText()) - if ack { - cc.Acknowledge(res.GetChatMessage().GetMessageId()) - } else { - //XXX - } - } else if res.GetChatAcknowledge() != nil { - cc.Handler.ChatMessageAck(res.GetChatMessage().GetMessageId()) + cc.Acknowledge(res.GetChatMessage().GetMessageId(), ack) + } else if ack := res.GetChatAcknowledge(); ack != nil { + cc.Handler.ChatMessageAck(ack.GetMessageId(), ack.GetAccepted()) } // XXX? } diff --git a/channels/chatchannel_test.go b/channels/chatchannel_test.go index b404258..8ea0b12 100644 --- a/channels/chatchannel_test.go +++ b/channels/chatchannel_test.go @@ -76,7 +76,7 @@ func (tcch *TestChatChannelHandler) ChatMessage(messageID uint32, when time.Time return true } -func (tcch *TestChatChannelHandler) ChatMessageAck(messageID uint32) { +func (tcch *TestChatChannelHandler) ChatMessageAck(messageID uint32, accepted bool) { } @@ -116,7 +116,7 @@ func TestChatChannelOperations(t *testing.T) { t.Errorf("After Successful Result ChatChannel Is Still Pending") } - chat := messageBuilder.ChatMessage("message text", 0) + chat := messageBuilder.ChatMessage("message text", 0, 0) chatChannel.Packet(chat) chatChannel.SendMessage("hello") diff --git a/examples/echobot/main.go b/examples/echobot/main.go index 2281a7c..a8d67f5 100644 --- a/examples/echobot/main.go +++ b/examples/echobot/main.go @@ -36,7 +36,7 @@ func (echobot *RicochetEchoBot) ChatMessage(messageID uint32, when time.Time, me return true } -func (echobot *RicochetEchoBot) ChatMessageAck(messageID uint32) { +func (echobot *RicochetEchoBot) ChatMessageAck(messageID uint32, accepted bool) { } diff --git a/utils/messagebuilder.go b/utils/messagebuilder.go index bd5616f..9b932f1 100644 --- a/utils/messagebuilder.go +++ b/utils/messagebuilder.go @@ -195,10 +195,11 @@ func (mb *MessageBuilder) AuthResult(accepted bool, isKnownContact bool) []byte } // ChatMessage constructs a chat message with the given content. -func (mb *MessageBuilder) ChatMessage(message string, messageID uint32) []byte { +func (mb *MessageBuilder) ChatMessage(message string, messageID uint32, timeDelta int64) []byte { cm := &Protocol_Data_Chat.ChatMessage{ MessageId: proto.Uint32(messageID), MessageText: proto.String(message), + TimeDelta: proto.Int64(timeDelta), } chatPacket := &Protocol_Data_Chat.Packet{ ChatMessage: cm, @@ -209,10 +210,10 @@ func (mb *MessageBuilder) ChatMessage(message string, messageID uint32) []byte { } // AckChatMessage constructs a chat message acknowledgement. -func (mb *MessageBuilder) AckChatMessage(messageID uint32) []byte { +func (mb *MessageBuilder) AckChatMessage(messageID uint32, accepted bool) []byte { cr := &Protocol_Data_Chat.ChatAcknowledge{ MessageId: proto.Uint32(messageID), - Accepted: proto.Bool(true), + Accepted: proto.Bool(accepted), } pc := &Protocol_Data_Chat.Packet{ ChatAcknowledge: cr, diff --git a/utils/messagebuilder_test.go b/utils/messagebuilder_test.go index cdd9f27..3f1c16a 100644 --- a/utils/messagebuilder_test.go +++ b/utils/messagebuilder_test.go @@ -26,7 +26,7 @@ func TestOpenAuthenticationChannel(t *testing.T) { func TestChatMessage(t *testing.T) { messageBuilder := new(MessageBuilder) - messageBuilder.ChatMessage("Hello World", 0) + messageBuilder.ChatMessage("Hello World", 0, 0) // TODO: More Indepth Test Of Output }