Skip to content

Commit

Permalink
feat(messages): History and offline message support for extended mess…
Browse files Browse the repository at this point in the history
…ages

* Added new negotiating friend state to allow delayed sending of offline
messages
* Added ability to flag currently outgoing message as broken in UI
* Reworked OfflineMsgEngine to support multiple receipt types
    * Moved resending logic out of the OfflineMsgEngine
    * Moved coordination of receipt and DispatchedMessageId into helper
    class usable for both ExtensionReceiptNum and ReceiptNum
    * Resending logic now has a failure case when the friend's extension
    set is lower than the required extensions needed for the message
    * When a user is known to be offline we do not allow use of any
    extensions
* Added DB support for broken message reasons
* Added DB support to tie an faux_offline_pending message to a required
extension set
  • Loading branch information
sphaerophoria authored and anthonybilinski committed Jan 30, 2021
1 parent 7474c6d commit 5f5f612
Show file tree
Hide file tree
Showing 38 changed files with 910 additions and 454 deletions.
10 changes: 10 additions & 0 deletions img/status/negotiating.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
32 changes: 32 additions & 0 deletions img/status/negotiating_notification.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 2 additions & 0 deletions res.qrc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
<file>img/status/away_notification.svg</file>
<file>img/status/busy.svg</file>
<file>img/status/busy_notification.svg</file>
<file>img/status/negotiating.svg</file>
<file>img/status/negotiating_notification.svg</file>
<file>img/status/offline.svg</file>
<file>img/status/offline_notification.svg</file>
<file>img/status/online.svg</file>
Expand Down
5 changes: 5 additions & 0 deletions src/chatlog/chatmessage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,11 @@ void ChatMessage::markAsDelivered(const QDateTime& time)
replaceContent(2, new Timestamp(time, Settings::getInstance().getTimestampFormat(), baseFont));
}

void ChatMessage::markAsBroken()
{
replaceContent(2, new Broken(Style::getImagePath("chatArea/error.svg"), QSize(16, 16)));
}

QString ChatMessage::toString() const
{
ChatLineContent* c = getContent(1);
Expand Down
1 change: 1 addition & 0 deletions src/chatlog/chatmessage.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class ChatMessage : public ChatLine
static ChatMessage::Ptr createBusyNotification();

void markAsDelivered(const QDateTime& time);
void markAsBroken();
QString toString() const;
bool isAction() const;
void setAsAction();
Expand Down
1 change: 1 addition & 0 deletions src/core/core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1096,6 +1096,7 @@ bool Core::sendMessageWithType(uint32_t friendId, const QString& message, Tox_Me
int size = message.toUtf8().size();
auto maxSize = static_cast<int>(tox_max_message_length());
if (size > maxSize) {
assert(false);
qCritical() << "Core::sendMessageWithType called with message of size:" << size
<< "when max is:" << maxSize << ". Ignoring.";
return false;
Expand Down
3 changes: 3 additions & 0 deletions src/core/receiptnum.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,6 @@

using ReceiptNum = NamedType<uint32_t, struct ReceiptNumTag, Orderable>;
Q_DECLARE_METATYPE(ReceiptNum)

using ExtendedReceiptNum = NamedType<uint32_t, struct ExtendedReceiptNumTag, Orderable>;
Q_DECLARE_METATYPE(ExtendedReceiptNum);
27 changes: 27 additions & 0 deletions src/model/brokenmessagereason.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
Copyright © 2015-2019 by The qTox Project Contributors
This file is part of qTox, a Qt-based graphical interface for Tox.
qTox is libre software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
qTox is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with qTox. If not, see <http://www.gnu.org/licenses/>.
*/

#pragma once

// NOTE: Numbers are important here as this is cast to an int and persisted in the DB
enum class BrokenMessageReason : int
{
unknown = 0,
unsupportedExtensions = 1
};
61 changes: 51 additions & 10 deletions src/model/chathistory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ ChatHistory::ChatHistory(Friend& f_, History* history_, const ICoreIdHandler& co
&ChatHistory::onMessageComplete);
connect(&messageDispatcher, &IMessageDispatcher::messageReceived, this,
&ChatHistory::onMessageReceived);
connect(&messageDispatcher, &IMessageDispatcher::messageBroken, this,
&ChatHistory::onMessageBroken);

if (canUseHistory()) {
// Defer messageSent callback until we finish firing off all our unsent messages.
Expand Down Expand Up @@ -266,7 +268,7 @@ void ChatHistory::onMessageReceived(const ToxPk& sender, const Message& message)
content = ChatForm::ACTION_PREFIX + content;
}

history->addNewMessage(friendPk, content, friendPk, message.timestamp, true, displayName);
history->addNewMessage(friendPk, content, friendPk, message.timestamp, true, message.extensionSet, displayName);
}

sessionChatLog.onMessageReceived(sender, message);
Expand All @@ -287,7 +289,7 @@ void ChatHistory::onMessageSent(DispatchedMessageId id, const Message& message)

auto onInsertion = [this, id](RowId historyId) { handleDispatchedMessage(id, historyId); };

history->addNewMessage(friendPk, content, selfPk, message.timestamp, false, username,
history->addNewMessage(friendPk, content, selfPk, message.timestamp, false, message.extensionSet, username,
onInsertion);
}

Expand All @@ -303,6 +305,15 @@ void ChatHistory::onMessageComplete(DispatchedMessageId id)
sessionChatLog.onMessageComplete(id);
}

void ChatHistory::onMessageBroken(DispatchedMessageId id, BrokenMessageReason reason)
{
if (canUseHistory()) {
breakMessage(id, reason);
}

sessionChatLog.onMessageBroken(id, reason);
}

/**
* @brief Forces the given index and all future indexes to be in the chatlog
* @param[in] idx
Expand Down Expand Up @@ -405,6 +416,13 @@ void ChatHistory::loadHistoryIntoSessionChatLog(ChatLogIdx start) const
void ChatHistory::dispatchUnsentMessages(IMessageDispatcher& messageDispatcher)
{
auto unsentMessages = history->getUndeliveredMessagesForFriend(f.getPublicKey());

auto requiredExtensions = std::accumulate(
unsentMessages.begin(), unsentMessages.end(),
ExtensionSet(), [] (const ExtensionSet& a, const History::HistMessage& b) {
return a | b.extensionSet;
});

for (auto& message : unsentMessages) {
// We should only store messages as unsent, if this changes in the
// future we need to extend this logic
Expand All @@ -418,12 +436,14 @@ void ChatHistory::dispatchUnsentMessages(IMessageDispatcher& messageDispatcher)
// with the new timestamp. This is intentional as everywhere else we use
// attempted send time (which is whenever the it was initially inserted
// into history
auto dispatchIds = messageDispatcher.sendMessage(isAction, messageContent);
auto dispatchId = requiredExtensions.none()
// We should only send a single message, but in the odd case where we end
// up having to split more than when we added the message to history we'll
// just associate the last dispatched id with the history message
? messageDispatcher.sendMessage(isAction, messageContent).second
: messageDispatcher.sendExtendedMessage(messageContent, requiredExtensions);

// We should only send a single message, but in the odd case where we end
// up having to split more than when we added the message to history we'll
// just associate the last dispatched id with the history message
handleDispatchedMessage(dispatchIds.second, message.id);
handleDispatchedMessage(dispatchId, message.id);

// We don't add the messages to the underlying chatlog since
// 1. We don't even know the ChatLogIdx of this message
Expand All @@ -435,11 +455,20 @@ void ChatHistory::dispatchUnsentMessages(IMessageDispatcher& messageDispatcher)
void ChatHistory::handleDispatchedMessage(DispatchedMessageId dispatchId, RowId historyId)
{
auto completedMessageIt = completedMessages.find(dispatchId);
if (completedMessageIt == completedMessages.end()) {
dispatchedMessageRowIdMap.insert(dispatchId, historyId);
} else {
auto brokenMessageIt = brokenMessages.find(dispatchId);

const auto isCompleted = completedMessageIt != completedMessages.end();
const auto isBroken = brokenMessageIt != brokenMessages.end();
assert(!(isCompleted && isBroken));

if (isCompleted) {
history->markAsDelivered(historyId);
completedMessages.erase(completedMessageIt);
} else if (isBroken) {
history->markAsBroken(historyId, brokenMessageIt.value());
brokenMessages.erase(brokenMessageIt);
} else {
dispatchedMessageRowIdMap.insert(dispatchId, historyId);
}
}

Expand All @@ -455,6 +484,18 @@ void ChatHistory::completeMessage(DispatchedMessageId id)
}
}

void ChatHistory::breakMessage(DispatchedMessageId id, BrokenMessageReason reason)
{
auto dispatchedMessageIt = dispatchedMessageRowIdMap.find(id);

if (dispatchedMessageIt == dispatchedMessageRowIdMap.end()) {
brokenMessages.insert(id, reason);
} else {
history->markAsBroken(*dispatchedMessageIt, reason);
dispatchedMessageRowIdMap.erase(dispatchedMessageIt);
}
}

bool ChatHistory::canUseHistory() const
{
return history && settings.getEnableLogging();
Expand Down
6 changes: 6 additions & 0 deletions src/model/chathistory.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

#include "ichatlog.h"
#include "sessionchatlog.h"
#include "src/model/brokenmessagereason.h"
#include "src/persistence/history.h"

#include <QSet>
Expand Down Expand Up @@ -51,13 +52,15 @@ private slots:
void onMessageReceived(const ToxPk& sender, const Message& message);
void onMessageSent(DispatchedMessageId id, const Message& message);
void onMessageComplete(DispatchedMessageId id);
void onMessageBroken(DispatchedMessageId id, BrokenMessageReason reason);

private:
void ensureIdxInSessionChatLog(ChatLogIdx idx) const;
void loadHistoryIntoSessionChatLog(ChatLogIdx start) const;
void dispatchUnsentMessages(IMessageDispatcher& messageDispatcher);
void handleDispatchedMessage(DispatchedMessageId dispatchId, RowId historyId);
void completeMessage(DispatchedMessageId id);
void breakMessage(DispatchedMessageId id, BrokenMessageReason reason);
bool canUseHistory() const;
ChatLogIdx getInitialChatLogIdx() const;

Expand All @@ -70,6 +73,9 @@ private slots:
// If a message completes before it's inserted into history it will end up
// in this set
QSet<DispatchedMessageId> completedMessages;
// If a message breaks before it's inserted into history it will end up
// in this set
QMap<DispatchedMessageId, BrokenMessageReason> brokenMessages;

// If a message is inserted into history before it gets a completion
// callback it will end up in this map
Expand Down
61 changes: 52 additions & 9 deletions src/model/friend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,17 @@
#include "src/persistence/profile.h"
#include "src/widget/form/chatform.h"

#include <QDebug>
#include <memory>

Friend::Friend(uint32_t friendId, const ToxPk& friendPk, const QString& userAlias, const QString& userName)
: userName{userName}
, userAlias{userAlias}
, friendPk{friendPk}
, friendId{friendId}
, hasNewEvents{false}
, friendStatus{Status::Status::Offline}
, isNegotiating{false}
{
if (userName.isEmpty()) {
this->userName = friendPk.toString();
Expand Down Expand Up @@ -151,22 +155,41 @@ bool Friend::getEventFlag() const

void Friend::setStatus(Status::Status s)
{
if (friendStatus != s) {
auto oldStatus = friendStatus;
friendStatus = s;
emit statusChanged(friendPk, friendStatus);
if (!Status::isOnline(oldStatus) && Status::isOnline(friendStatus)) {
emit onlineOfflineChanged(friendPk, true);
} else if (Status::isOnline(oldStatus) && !Status::isOnline(friendStatus)) {
// Internal status should never be negotiating. We only expose this externally through the use of isNegotiating
assert(s != Status::Status::Negotiating);

const bool wasOnline = Status::isOnline(getStatus());
if (friendStatus == s) {
return;
}

// When a friend goes online we want to give them some time to negotiate
// extension support
const auto startNegotating = friendStatus == Status::Status::Offline;

if (startNegotating) {
qDebug() << "Starting negotiation with friend " << friendId;
isNegotiating = true;
}

friendStatus = s;
const bool nowOnline = Status::isOnline(getStatus());

const auto emitStatusChange = startNegotating || !isNegotiating;
if (emitStatusChange) {
const auto statusToEmit = isNegotiating ? Status::Status::Negotiating : friendStatus;
emit statusChanged(friendPk, statusToEmit);
if (wasOnline && !nowOnline) {
emit onlineOfflineChanged(friendPk, false);
} else if (!wasOnline && nowOnline) {
emit onlineOfflineChanged(friendPk, true);
}

}
}

Status::Status Friend::getStatus() const
{
return friendStatus;
return isNegotiating ? Status::Status::Negotiating : friendStatus;
}

bool Friend::useHistory() const
Expand All @@ -178,9 +201,29 @@ void Friend::setExtendedMessageSupport(bool supported)
{
supportedExtensions[ExtensionType::messages] = supported;
emit extensionSupportChanged(supportedExtensions);

// If all extensions are supported we can exit early
if (supportedExtensions.all()) {
onNegotiationComplete();
}
}

ExtensionSet Friend::getSupportedExtensions() const
{
return supportedExtensions;
}

void Friend::onNegotiationComplete() {
if (!isNegotiating) {
return;
}

qDebug() << "Negotiation complete for friend " << friendId;

isNegotiating = false;
emit statusChanged(friendPk, friendStatus);

if (Status::isOnline(getStatus())) {
emit onlineOfflineChanged(friendPk, true);
}
}
2 changes: 2 additions & 0 deletions src/model/friend.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class Friend : public Contact
void loadChatHistory();

public slots:
void onNegotiationComplete();
private:
QString userName;
QString userAlias;
Expand All @@ -77,5 +78,6 @@ public slots:
uint32_t friendId;
bool hasNewEvents;
Status::Status friendStatus;
bool isNegotiating;
ExtensionSet supportedExtensions;
};
Loading

0 comments on commit 5f5f612

Please sign in to comment.