diff --git a/CHANGELOG.md b/CHANGELOG.md index 11cd233788d..7cee048904a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ - Bugfix: Fixed tab visibility being controllable in the emote popup. (#5530) - Bugfix: Fixed account switch not being saved if no other settings were changed. (#5558) - Bugfix: Fixed some tooltips not being readable. (#5578) +- Bugfix: Fixed log files being locked longer than needed. (#5592) - Dev: Update Windows build from Qt 6.5.0 to Qt 6.7.1. (#5420) - Dev: Update vcpkg build Qt from 6.5.0 to 6.7.0, boost from 1.83.0 to 1.85.0, openssl from 3.1.3 to 3.3.0. (#5422) - Dev: Unsingletonize `ISoundController`. (#5462) diff --git a/mocks/include/mocks/Logging.hpp b/mocks/include/mocks/Logging.hpp index 0f4dba38a18..78c77ef0c6b 100644 --- a/mocks/include/mocks/Logging.hpp +++ b/mocks/include/mocks/Logging.hpp @@ -18,6 +18,10 @@ class Logging : public ILogging (const QString &channelName, MessagePtr message, const QString &platformName, const QString &streamID), (override)); + + MOCK_METHOD(void, closeChannel, + (const QString &channelName, const QString &platformName), + (override)); }; class EmptyLogging : public ILogging @@ -32,6 +36,12 @@ class EmptyLogging : public ILogging { // } + + void closeChannel(const QString &channelName, + const QString &platformName) override + { + // + } }; } // namespace chatterino::mock diff --git a/src/Application.cpp b/src/Application.cpp index d781dbac7de..8cb2211015b 100644 --- a/src/Application.cpp +++ b/src/Application.cpp @@ -149,6 +149,7 @@ Application::Application(Settings &_settings, const Paths &paths, , args_(_args) , themes(new Theme(paths)) , fonts(new Fonts(_settings)) + , logging(new Logging(_settings)) , emotes(new Emotes) , accounts(new AccountController) , hotkeys(new HotkeyController) @@ -175,7 +176,6 @@ Application::Application(Settings &_settings, const Paths &paths, , ffzEmotes(new FfzEmotes) , seventvEmotes(new SeventvEmotes) , seventvEventAPI(makeSeventvEventAPI(_settings)) - , logging(new Logging(_settings)) , linkResolver(new LinkResolver) , streamerMode(new StreamerMode) , twitchUsers(new TwitchUsers) @@ -503,6 +503,7 @@ PubSub *Application::getTwitchPubSub() ILogging *Application::getChatLogger() { assertInGuiThread(); + assert(this->logging); return this->logging.get(); } @@ -697,4 +698,9 @@ IApplication *getApp() return INSTANCE; } +IApplication *tryGetApp() +{ + return INSTANCE; +} + } // namespace chatterino diff --git a/src/Application.hpp b/src/Application.hpp index 2f6b11a3f33..71145b162a4 100644 --- a/src/Application.hpp +++ b/src/Application.hpp @@ -144,6 +144,7 @@ class Application : public IApplication private: std::unique_ptr themes; std::unique_ptr fonts; + const std::unique_ptr logging; std::unique_ptr emotes; std::unique_ptr accounts; std::unique_ptr hotkeys; @@ -169,7 +170,6 @@ class Application : public IApplication std::unique_ptr ffzEmotes; std::unique_ptr seventvEmotes; std::unique_ptr seventvEventAPI; - const std::unique_ptr logging; std::unique_ptr linkResolver; std::unique_ptr streamerMode; std::unique_ptr twitchUsers; @@ -239,4 +239,7 @@ class Application : public IApplication IApplication *getApp(); +/// Might return `nullptr` if the app is being destroyed +IApplication *tryGetApp(); + } // namespace chatterino diff --git a/src/common/Channel.cpp b/src/common/Channel.cpp index a74aa32c49e..ef778bad15a 100644 --- a/src/common/Channel.cpp +++ b/src/common/Channel.cpp @@ -38,6 +38,11 @@ Channel::Channel(const QString &name, Type type) Channel::~Channel() { + auto *app = tryGetApp(); + if (app) + { + app->getChatLogger()->closeChannel(this->name_, this->platform_); + } this->destroyed.invoke(); } diff --git a/src/singletons/Logging.cpp b/src/singletons/Logging.cpp index 138efd38a4c..7fef6c299e0 100644 --- a/src/singletons/Logging.cpp +++ b/src/singletons/Logging.cpp @@ -2,7 +2,6 @@ #include "messages/Message.hpp" #include "singletons/helper/LoggingChannel.hpp" -#include "singletons/Paths.hpp" #include "singletons/Settings.hpp" #include @@ -58,7 +57,7 @@ void Logging::addMessage(const QString &channelName, MessagePtr message, auto map = std::map>(); this->loggingChannels_[platformName] = std::move(map); auto &ref = this->loggingChannels_.at(platformName); - ref.emplace(channelName, std::move(channel)); + ref.emplace(channelName, channel); return; } auto chanIt = platIt->second.find(channelName); @@ -66,7 +65,7 @@ void Logging::addMessage(const QString &channelName, MessagePtr message, { auto *channel = new LoggingChannel(channelName, platformName); channel->addMessage(message, streamID); - platIt->second.emplace(channelName, std::move(channel)); + platIt->second.emplace(channelName, channel); } else { @@ -74,4 +73,15 @@ void Logging::addMessage(const QString &channelName, MessagePtr message, } } +void Logging::closeChannel(const QString &channelName, + const QString &platformName) +{ + auto platIt = this->loggingChannels_.find(platformName); + if (platIt == this->loggingChannels_.end()) + { + return; + } + platIt->second.erase(channelName); +} + } // namespace chatterino diff --git a/src/singletons/Logging.hpp b/src/singletons/Logging.hpp index 984932ad328..0af72386ee2 100644 --- a/src/singletons/Logging.hpp +++ b/src/singletons/Logging.hpp @@ -24,6 +24,9 @@ class ILogging virtual void addMessage(const QString &channelName, MessagePtr message, const QString &platformName, const QString &streamID) = 0; + + virtual void closeChannel(const QString &channelName, + const QString &platformName) = 0; }; class Logging : public ILogging @@ -35,6 +38,9 @@ class Logging : public ILogging const QString &platformName, const QString &streamID) override; + void closeChannel(const QString &channelName, + const QString &platformName) override; + private: using PlatformName = QString; using ChannelName = QString; diff --git a/src/singletons/helper/LoggingChannel.cpp b/src/singletons/helper/LoggingChannel.cpp index dcce92f234d..c27cb4c3a9c 100644 --- a/src/singletons/helper/LoggingChannel.cpp +++ b/src/singletons/helper/LoggingChannel.cpp @@ -40,7 +40,7 @@ QString generateClosingString( { QString ret("# Stop logging at "); - ret.append(now.toString("yyyy-MM-dd HH:mm:ss")); + ret.append(now.toString("yyyy-MM-dd HH:mm:ss ")); ret.append(now.timeZoneAbbreviation()); ret.append(ENDLINE); diff --git a/tests/src/ChannelChatters.cpp b/tests/src/ChannelChatters.cpp index 79711ce15af..f3975ec1653 100644 --- a/tests/src/ChannelChatters.cpp +++ b/tests/src/ChannelChatters.cpp @@ -1,6 +1,8 @@ #include "common/ChannelChatters.hpp" +#include "mocks/BaseApplication.hpp" #include "mocks/Channel.hpp" +#include "mocks/Logging.hpp" #include "Test.hpp" #include @@ -9,9 +11,28 @@ using namespace chatterino; using chatterino::mock::MockChannel; +namespace { + +class MockApplication : public mock::BaseApplication +{ +public: + MockApplication() = default; + + ILogging *getChatLogger() override + { + return &this->logging; + } + + mock::EmptyLogging logging; +}; + +} // namespace + // Ensure inserting the same user does not increase the size of the stored colors -TEST(ChatterChatters, insertSameUser) +TEST(ChannelChatters, insertSameUser) { + MockApplication app; + MockChannel channel("test"); ChannelChatters chatters(channel); @@ -24,8 +45,10 @@ TEST(ChatterChatters, insertSameUser) } // Ensure we can update a chatters color -TEST(ChatterChatters, insertSameUserUpdatesColor) +TEST(ChannelChatters, insertSameUserUpdatesColor) { + MockApplication app; + MockChannel channel("test"); ChannelChatters chatters(channel); @@ -37,8 +60,10 @@ TEST(ChatterChatters, insertSameUserUpdatesColor) } // Ensure getting a non-existant users color returns an invalid QColor -TEST(ChatterChatters, getNonExistantUser) +TEST(ChannelChatters, getNonExistantUser) { + MockApplication app; + MockChannel channel("test"); ChannelChatters chatters(channel); @@ -47,8 +72,10 @@ TEST(ChatterChatters, getNonExistantUser) } // Ensure getting a user doesn't create an entry -TEST(ChatterChatters, getDoesNotCreate) +TEST(ChannelChatters, getDoesNotCreate) { + MockApplication app; + MockChannel channel("test"); ChannelChatters chatters(channel); @@ -59,8 +86,10 @@ TEST(ChatterChatters, getDoesNotCreate) } // Ensure the least recently used entry is purged when we reach MAX_SIZE -TEST(ChatterChatters, insertMaxSize) +TEST(ChannelChatters, insertMaxSize) { + MockApplication app; + MockChannel channel("test"); ChannelChatters chatters(channel); diff --git a/tests/src/Commands.cpp b/tests/src/Commands.cpp index 420f71fdf5c..f1b872b7dc1 100644 --- a/tests/src/Commands.cpp +++ b/tests/src/Commands.cpp @@ -55,11 +55,11 @@ class MockApplication : public mock::BaseApplication return &this->chatLogger; } + mock::EmptyLogging chatLogger; AccountController accounts; CommandController commands; mock::MockTwitchIrcServer twitch; Emotes emotes; - mock::EmptyLogging chatLogger; }; } // namespace diff --git a/tests/src/Filters.cpp b/tests/src/Filters.cpp index dd592b3b6ee..af230454619 100644 --- a/tests/src/Filters.cpp +++ b/tests/src/Filters.cpp @@ -8,6 +8,7 @@ #include "mocks/Channel.hpp" #include "mocks/ChatterinoBadges.hpp" #include "mocks/EmptyApplication.hpp" +#include "mocks/Logging.hpp" #include "mocks/TwitchIrcServer.hpp" #include "mocks/UserData.hpp" #include "providers/ffz/FfzBadges.hpp" @@ -75,6 +76,12 @@ class MockApplication : public mock::BaseApplication return &this->highlights; } + ILogging *getChatLogger() override + { + return &this->logging; + } + + mock::EmptyLogging logging; AccountController accounts; Emotes emotes; mock::UserDataController userData; diff --git a/tests/src/InputCompletion.cpp b/tests/src/InputCompletion.cpp index 460fb152630..1f219570c49 100644 --- a/tests/src/InputCompletion.cpp +++ b/tests/src/InputCompletion.cpp @@ -8,6 +8,7 @@ #include "mocks/BaseApplication.hpp" #include "mocks/Channel.hpp" #include "mocks/Helix.hpp" +#include "mocks/Logging.hpp" #include "mocks/TwitchIrcServer.hpp" #include "singletons/Emotes.hpp" #include "singletons/Paths.hpp" @@ -69,6 +70,12 @@ class MockApplication : public mock::BaseApplication return &this->seventvEmotes; } + ILogging *getChatLogger() override + { + return &this->logging; + } + + mock::EmptyLogging logging; AccountController accounts; mock::MockTwitchIrcServer twitch; Emotes emotes; @@ -133,9 +140,9 @@ class InputCompletionTest : public ::testing::Test void TearDown() override { - this->mockApplication.reset(); - this->mockHelix.reset(); this->channelPtr.reset(); + this->mockHelix.reset(); + this->mockApplication.reset(); } std::unique_ptr mockApplication; diff --git a/tests/src/MessageBuilder.cpp b/tests/src/MessageBuilder.cpp index 39f01f196a4..9a3030822fd 100644 --- a/tests/src/MessageBuilder.cpp +++ b/tests/src/MessageBuilder.cpp @@ -7,6 +7,7 @@ #include "mocks/Channel.hpp" #include "mocks/ChatterinoBadges.hpp" #include "mocks/DisabledStreamerMode.hpp" +#include "mocks/Logging.hpp" #include "mocks/TwitchIrcServer.hpp" #include "mocks/UserData.hpp" #include "providers/ffz/FfzBadges.hpp" @@ -91,6 +92,12 @@ class MockApplication : public mock::BaseApplication return &this->seventvEmotes; } + ILogging *getChatLogger() override + { + return &this->logging; + } + + mock::EmptyLogging logging; AccountController accounts; Emotes emotes; mock::UserDataController userData;