Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor recent messages API #4763

Merged
merged 5 commits into from
Aug 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
- Dev: Replace our QObjectRef class with Qt's QPointer class. (#4666)
- Dev: Fixed warnings about QWidgets already having a QLayout. (#4672)
- Dev: Fixed undefined behavior when loading non-existant credentials. (#4673)
- Dev: Small refactor of the recent-messages API, splitting its internal API and its internal implementation up into separate files. (#4763)
- Dev: Added support for compiling with `sccache`. (#4678)
- Dev: Added `sccache` in Windows CI. (#4678)
- Dev: Moved preprocessor Git and date definitions to executables only. (#4681)
Expand Down
7 changes: 5 additions & 2 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,6 @@ set(SOURCE_FILES
providers/LinkResolver.hpp
providers/NetworkConfigurationProvider.cpp
providers/NetworkConfigurationProvider.hpp
providers/RecentMessagesApi.cpp
providers/RecentMessagesApi.hpp

providers/bttv/BttvEmotes.cpp
providers/bttv/BttvEmotes.hpp
Expand Down Expand Up @@ -288,6 +286,11 @@ set(SOURCE_FILES
providers/liveupdates/BasicPubSubManager.hpp
providers/liveupdates/BasicPubSubWebsocket.hpp

providers/recentmessages/Api.cpp
providers/recentmessages/Api.hpp
providers/recentmessages/Impl.cpp
providers/recentmessages/Impl.hpp

providers/seventv/SeventvAPI.cpp
providers/seventv/SeventvAPI.hpp
providers/seventv/SeventvBadges.cpp
Expand Down
240 changes: 0 additions & 240 deletions src/providers/RecentMessagesApi.cpp

This file was deleted.

33 changes: 0 additions & 33 deletions src/providers/RecentMessagesApi.hpp

This file was deleted.

91 changes: 91 additions & 0 deletions src/providers/recentmessages/Api.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
#include "providers/recentmessages/Api.hpp"

#include "common/NetworkRequest.hpp"
#include "common/NetworkResult.hpp"
#include "common/QLogging.hpp"
#include "providers/recentmessages/Impl.hpp"
#include "providers/twitch/TwitchMessageBuilder.hpp"
#include "util/PostToThread.hpp"

namespace {

// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
const auto &LOG = chatterinoRecentMessages;

} // namespace

namespace chatterino::recentmessages {

using namespace recentmessages::detail;

void load(const QString &channelName, std::weak_ptr<Channel> channelPtr,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: the parameter 'channelPtr' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]

Suggested change
void load(const QString &channelName, std::weak_ptr<Channel> channelPtr,
void load(const QString &channelName, const std::weak_ptr<Channel>& channelPtr,

src/providers/recentmessages/Api.hpp:31:

- void load(const QString &channelName, std::weak_ptr<Channel> channelPtr,
+ void load(const QString &channelName, const std::weak_ptr<Channel>& channelPtr,

ResultCallback onLoaded, ErrorCallback onError)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: the parameter 'onLoaded' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]

Suggested change
ResultCallback onLoaded, ErrorCallback onError)
const ResultCallback& onLoaded, ErrorCallback onError)

src/providers/recentmessages/Api.hpp:32:

-           ResultCallback onLoaded, ErrorCallback onError);
+           const ResultCallback& onLoaded, ErrorCallback onError);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: the parameter 'onError' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]

Suggested change
ResultCallback onLoaded, ErrorCallback onError)
ResultCallback onLoaded, const ErrorCallback& onError)

src/providers/recentmessages/Api.hpp:32:

-           ResultCallback onLoaded, ErrorCallback onError);
+           ResultCallback onLoaded, const ErrorCallback& onError);

{
qCDebug(LOG) << "Loading recent messages for" << channelName;

const auto url = constructRecentMessagesUrl(channelName);

NetworkRequest(url)
.onSuccess([channelPtr, onLoaded](const auto &result) -> Outcome {
auto shared = channelPtr.lock();
if (!shared)
{
return Failure;
}

qCDebug(LOG) << "Successfully loaded recent messages for"
<< shared->getName();

auto root = result.parseJson();
auto parsedMessages = parseRecentMessages(root);

// build the Communi messages into chatterino messages
auto builtMessages =
buildRecentMessages(parsedMessages, shared.get());

postToThread([shared = std::move(shared), root = std::move(root),
messages = std::move(builtMessages),
onLoaded]() mutable {
// Notify user about a possible gap in logs if it returned some messages
// but isn't currently joined to a channel
const auto errorCode = root.value("error_code").toString();
if (!errorCode.isEmpty())
{
qCDebug(LOG)
<< QString("Got error from API: error_code=%1, "
"channel=%2")
.arg(errorCode, shared->getName());
if (errorCode == "channel_not_joined" && !messages.empty())
{
shared->addMessage(makeSystemMessage(
"Message history service recovering, there may "
"be gaps in the message history."));
}
}

onLoaded(messages);
});

return Success;
})
.onError([channelPtr, onError](const NetworkResult &result) {
auto shared = channelPtr.lock();
if (!shared)
{
return;
}

qCDebug(LOG) << "Failed to load recent messages for"
<< shared->getName();

shared->addMessage(makeSystemMessage(
QStringLiteral(
"Message history service unavailable (Error: %1)")
.arg(result.formatError())));

onError();
})
.execute();
}

} // namespace chatterino::recentmessages
Loading
Loading