From 94643762cfbf662be229c527abeffd9b6443df02 Mon Sep 17 00:00:00 2001 From: Matej Kenda Date: Thu, 27 Nov 2025 10:31:23 +0100 Subject: [PATCH] enh(MongoDB): simplified and modernised code. --- MongoDB/include/Poco/MongoDB/ReadPreference.h | 4 +- MongoDB/include/Poco/MongoDB/ReplicaSet.h | 19 ++-- .../include/Poco/MongoDB/ServerDescription.h | 6 +- .../Poco/MongoDB/TopologyDescription.h | 2 +- MongoDB/src/ReadPreference.cpp | 60 +++------- MongoDB/src/ReplicaSet.cpp | 56 ++------- MongoDB/src/ReplicaSetConnection.cpp | 12 +- MongoDB/src/ServerDescription.cpp | 107 +++--------------- MongoDB/src/TopologyDescription.cpp | 57 ++++------ 9 files changed, 76 insertions(+), 247 deletions(-) diff --git a/MongoDB/include/Poco/MongoDB/ReadPreference.h b/MongoDB/include/Poco/MongoDB/ReadPreference.h index 4bf1220b4..0922275da 100644 --- a/MongoDB/include/Poco/MongoDB/ReadPreference.h +++ b/MongoDB/include/Poco/MongoDB/ReadPreference.h @@ -135,9 +135,9 @@ private: std::vector filterByMaxStaleness(const std::vector& servers, const ServerDescription& primary) const; std::vector selectByNearest(const std::vector& servers) const; - Mode _mode; + Mode _mode{Primary}; Document _tags; - Poco::Int64 _maxStalenessSeconds; + Poco::Int64 _maxStalenessSeconds{NO_MAX_STALENESS}; }; diff --git a/MongoDB/include/Poco/MongoDB/ReplicaSet.h b/MongoDB/include/Poco/MongoDB/ReplicaSet.h index f5ff6785e..f08a9dbc8 100644 --- a/MongoDB/include/Poco/MongoDB/ReplicaSet.h +++ b/MongoDB/include/Poco/MongoDB/ReplicaSet.h @@ -81,26 +81,23 @@ public: /// Expected replica set name. /// If empty, will be discovered from servers. - ReadPreference readPreference; + ReadPreference readPreference{ReadPreference::Primary}; /// Default read preference for this replica set. - Poco::Timespan connectTimeout; + Poco::Timespan connectTimeout{10, 0}; /// Connection timeout (default: 10 seconds) - Poco::Timespan socketTimeout; + Poco::Timespan socketTimeout{30, 0}; /// Socket send/receive timeout (default: 30 seconds) - Poco::Timespan heartbeatFrequency; + Poco::Timespan heartbeatFrequency{10, 0}; /// Topology monitoring interval (default: 10 seconds) - bool enableMonitoring; + bool enableMonitoring{true}; /// Enable background topology monitoring (default: true) - Connection::SocketFactory* socketFactory; + Connection::SocketFactory* socketFactory{nullptr}; /// Optional socket factory for SSL/TLS connections - - Config(); - /// Creates default configuration. }; explicit ReplicaSet(const Config& config); @@ -203,8 +200,8 @@ private: TopologyDescription _topology; mutable Poco::FastMutex _mutex; std::thread _monitorThread; - std::atomic _stopMonitoring; - std::atomic _monitoringActive; + std::atomic _stopMonitoring{false}; + std::atomic _monitoringActive{false}; }; diff --git a/MongoDB/include/Poco/MongoDB/ServerDescription.h b/MongoDB/include/Poco/MongoDB/ServerDescription.h index e862a4aa8..e7d3120e8 100644 --- a/MongoDB/include/Poco/MongoDB/ServerDescription.h +++ b/MongoDB/include/Poco/MongoDB/ServerDescription.h @@ -140,14 +140,14 @@ private: void parseTags(const Document& doc); Net::SocketAddress _address; - ServerType _type; + ServerType _type{Unknown}; Timestamp _lastUpdateTime; - Poco::Int64 _roundTripTime; + Poco::Int64 _roundTripTime{0}; std::string _setName; std::vector _hosts; Document _tags; std::string _error; - bool _hasError; + bool _hasError{false}; }; diff --git a/MongoDB/include/Poco/MongoDB/TopologyDescription.h b/MongoDB/include/Poco/MongoDB/TopologyDescription.h index 9aed66235..eaaa168ad 100644 --- a/MongoDB/include/Poco/MongoDB/TopologyDescription.h +++ b/MongoDB/include/Poco/MongoDB/TopologyDescription.h @@ -146,7 +146,7 @@ private: /// Must be called while holding the mutex. mutable Mutex _mutex; - TopologyType _type; + TopologyType _type{Unknown}; std::string _setName; std::map _servers; }; diff --git a/MongoDB/src/ReadPreference.cpp b/MongoDB/src/ReadPreference.cpp index 0a12ca904..97205301a 100644 --- a/MongoDB/src/ReadPreference.cpp +++ b/MongoDB/src/ReadPreference.cpp @@ -28,9 +28,7 @@ const Poco::Int64 ReadPreference::NO_MAX_STALENESS; ReadPreference::ReadPreference(Mode mode): - _mode(mode), - _tags(), - _maxStalenessSeconds(NO_MAX_STALENESS) + _mode(mode) { } @@ -43,54 +41,19 @@ ReadPreference::ReadPreference(Mode mode, const Document& tags, Poco::Int64 maxS } -ReadPreference::ReadPreference(const ReadPreference& other): - _mode(other._mode), - _tags(other._tags), - _maxStalenessSeconds(other._maxStalenessSeconds) -{ -} +ReadPreference::ReadPreference(const ReadPreference& other) = default; -ReadPreference::ReadPreference(ReadPreference&& other) noexcept: - _mode(other._mode), - _tags(std::move(other._tags)), - _maxStalenessSeconds(other._maxStalenessSeconds) -{ - other._mode = Primary; - other._maxStalenessSeconds = NO_MAX_STALENESS; -} +ReadPreference::ReadPreference(ReadPreference&& other) noexcept = default; -ReadPreference::~ReadPreference() -{ -} +ReadPreference::~ReadPreference() = default; -ReadPreference& ReadPreference::operator=(const ReadPreference& other) -{ - if (this != &other) - { - _mode = other._mode; - _tags = other._tags; - _maxStalenessSeconds = other._maxStalenessSeconds; - } - return *this; -} +ReadPreference& ReadPreference::operator=(const ReadPreference& other) = default; -ReadPreference& ReadPreference::operator=(ReadPreference&& other) noexcept -{ - if (this != &other) - { - _mode = other._mode; - _tags = std::move(other._tags); - _maxStalenessSeconds = other._maxStalenessSeconds; - - other._mode = Primary; - other._maxStalenessSeconds = NO_MAX_STALENESS; - } - return *this; -} +ReadPreference& ReadPreference::operator=(ReadPreference&& other) noexcept = default; std::vector ReadPreference::selectServers(const TopologyDescription& topology) const @@ -251,8 +214,8 @@ bool ReadPreference::matchesTags(const ServerDescription& server) const } // Get both values as strings for comparison - std::string requiredValue = _tags.get(key); - std::string serverValue = serverTags.get(key); + const auto& requiredValue = _tags.get(key); + const auto& serverValue = serverTags.get(key); if (requiredValue != serverValue) { @@ -272,6 +235,7 @@ std::vector ReadPreference::filterByTags(const std::vector result; + result.reserve(servers.size()); for (const auto& server : servers) { if (matchesTags(server)) @@ -297,7 +261,8 @@ std::vector ReadPreference::filterByMaxStaleness( // A full implementation would compare lastWriteDate timestamps std::vector result; - Poco::Int64 maxStalenessMs = _maxStalenessSeconds * 1000000; // Convert to microseconds + result.reserve(servers.size()); + const Poco::Int64 maxStalenessMs = _maxStalenessSeconds * 1000000; // Convert to microseconds for (const auto& server : servers) { @@ -346,12 +311,13 @@ std::vector ReadPreference::selectByNearest(const std::vector // Select servers within 15ms of minimum RTT (MongoDB spec) const Poco::Int64 localThresholdMs = 15000; // 15ms in microseconds std::vector result; + result.reserve(servers.size()); for (const auto& server : servers) { if (server.roundTripTime() <= minRTT + localThresholdMs) { - result.push_back(server); + result.emplace_back(server); } } diff --git a/MongoDB/src/ReplicaSet.cpp b/MongoDB/src/ReplicaSet.cpp index b24dd15ab..0b32f2b28 100644 --- a/MongoDB/src/ReplicaSet.cpp +++ b/MongoDB/src/ReplicaSet.cpp @@ -26,24 +26,6 @@ namespace Poco { namespace MongoDB { -// -// ReplicaSet::Config -// - - -ReplicaSet::Config::Config(): - seeds(), - setName(), - readPreference(ReadPreference::Primary), - connectTimeout(10, 0), // 10 seconds - socketTimeout(30, 0), // 30 seconds - heartbeatFrequency(10, 0), // 10 seconds - enableMonitoring(true), - socketFactory(nullptr) -{ -} - - // // ReplicaSet // @@ -51,11 +33,7 @@ ReplicaSet::Config::Config(): ReplicaSet::ReplicaSet(const Config& config): _config(config), - _topology(config.setName), - _mutex(), - _monitorThread(), - _stopMonitoring(false), - _monitoringActive(false) + _topology(config.setName) { if (_config.seeds.empty()) { @@ -106,13 +84,7 @@ ReplicaSet::ReplicaSet(const std::vector& seeds): } -ReplicaSet::ReplicaSet(const std::string& uri): - _config(), - _topology(), - _mutex(), - _monitorThread(), - _stopMonitoring(false), - _monitoringActive(false) +ReplicaSet::ReplicaSet(const std::string& uri) { // Parse URI first to extract seeds and configuration parseURI(uri); @@ -287,7 +259,7 @@ void ReplicaSet::discover() // Try to discover topology from seed servers bool discovered = false; - std::vector servers = _topology.servers(); + const auto servers = _topology.servers(); for (const auto& server : servers) { try @@ -510,19 +482,11 @@ void ReplicaSet::parseURI(const std::string& uri) // Parse authority to extract multiple hosts // The authority in MongoDB URIs can be: host1:port1,host2:port2,host3:port3 // Poco::URI will give us the full authority string, we need to parse it manually - std::string authority = theURI.getAuthority(); + const auto& authority = theURI.getAuthority(); // Remove userinfo if present (username:password@) - std::string::size_type atPos = authority.find('@'); - std::string hostsStr; - if (atPos != std::string::npos) - { - hostsStr = authority.substr(atPos + 1); - } - else - { - hostsStr = authority; - } + const auto atPos = authority.find('@'); + const auto hostsStr = (atPos != std::string::npos) ? authority.substr(atPos + 1) : authority; // Parse comma-separated hosts _config.seeds.clear(); @@ -531,12 +495,12 @@ void ReplicaSet::parseURI(const std::string& uri) while ((end = hostsStr.find(',', start)) != std::string::npos) { - std::string hostPort = hostsStr.substr(start, end - start); + const auto hostPort = hostsStr.substr(start, end - start); if (!hostPort.empty()) { try { - _config.seeds.push_back(Net::SocketAddress(hostPort)); + _config.seeds.emplace_back(hostPort); } catch (...) { @@ -547,12 +511,12 @@ void ReplicaSet::parseURI(const std::string& uri) } // Parse last host - std::string lastHost = hostsStr.substr(start); + const auto lastHost = hostsStr.substr(start); if (!lastHost.empty()) { try { - _config.seeds.push_back(Net::SocketAddress(lastHost)); + _config.seeds.emplace_back(lastHost); } catch (...) { diff --git a/MongoDB/src/ReplicaSetConnection.cpp b/MongoDB/src/ReplicaSetConnection.cpp index c278e181f..26fa8bd19 100644 --- a/MongoDB/src/ReplicaSetConnection.cpp +++ b/MongoDB/src/ReplicaSetConnection.cpp @@ -41,16 +41,12 @@ namespace ErrorCodes ReplicaSetConnection::ReplicaSetConnection(ReplicaSet& replicaSet, const ReadPreference& readPref): _replicaSet(replicaSet), - _readPreference(readPref), - _connection(), - _triedServers() + _readPreference(readPref) { } -ReplicaSetConnection::~ReplicaSetConnection() -{ -} +ReplicaSetConnection::~ReplicaSetConnection() = default; void ReplicaSetConnection::sendRequest(OpMsgMessage& request, OpMsgMessage& response) @@ -243,7 +239,7 @@ bool ReplicaSetConnection::isRetriableError(const std::exception& e) const Poco::IOException* ioEx = dynamic_cast(&e); if (ioEx) { - std::string msg = ioEx->message(); + const auto& msg = ioEx->message(); // Check for specific retriable error messages if (msg.find("not master") != std::string::npos || msg.find("NotMaster") != std::string::npos || @@ -294,7 +290,7 @@ bool ReplicaSetConnection::isRetriableMongoDBError(const OpMsgMessage& response) // Check for error message patterns if (body.exists("errmsg")) { - std::string errmsg = body.get("errmsg"); + const auto& errmsg = body.get("errmsg"); if (errmsg.find("not master") != std::string::npos || errmsg.find("NotMaster") != std::string::npos) { diff --git a/MongoDB/src/ServerDescription.cpp b/MongoDB/src/ServerDescription.cpp index 4d6f60cd2..4d27eb50d 100644 --- a/MongoDB/src/ServerDescription.cpp +++ b/MongoDB/src/ServerDescription.cpp @@ -22,108 +22,28 @@ namespace Poco { namespace MongoDB { -ServerDescription::ServerDescription(): - _address(), - _type(Unknown), - _lastUpdateTime(), - _roundTripTime(0), - _setName(), - _hosts(), - _tags(), - _error(), - _hasError(false) -{ -} +ServerDescription::ServerDescription() = default; ServerDescription::ServerDescription(const Net::SocketAddress& address): - _address(address), - _type(Unknown), - _lastUpdateTime(), - _roundTripTime(0), - _setName(), - _hosts(), - _tags(), - _error(), - _hasError(false) + _address(address) { } -ServerDescription::ServerDescription(const ServerDescription& other): - _address(other._address), - _type(other._type), - _lastUpdateTime(other._lastUpdateTime), - _roundTripTime(other._roundTripTime), - _setName(other._setName), - _hosts(other._hosts), - _tags(other._tags), - _error(other._error), - _hasError(other._hasError) -{ -} +ServerDescription::ServerDescription(const ServerDescription& other) = default; -ServerDescription::ServerDescription(ServerDescription&& other) noexcept: - _address(std::move(other._address)), - _type(other._type), - _lastUpdateTime(other._lastUpdateTime), - _roundTripTime(other._roundTripTime), - _setName(std::move(other._setName)), - _hosts(std::move(other._hosts)), - _tags(std::move(other._tags)), - _error(std::move(other._error)), - _hasError(other._hasError) -{ - other._type = Unknown; - other._roundTripTime = 0; - other._hasError = false; -} +ServerDescription::ServerDescription(ServerDescription&& other) noexcept = default; -ServerDescription::~ServerDescription() -{ -} +ServerDescription::~ServerDescription() = default; -ServerDescription& ServerDescription::operator=(const ServerDescription& other) -{ - if (this != &other) - { - _address = other._address; - _type = other._type; - _lastUpdateTime = other._lastUpdateTime; - _roundTripTime = other._roundTripTime; - _setName = other._setName; - _hosts = other._hosts; - _tags = other._tags; - _error = other._error; - _hasError = other._hasError; - } - return *this; -} +ServerDescription& ServerDescription::operator=(const ServerDescription& other) = default; -ServerDescription& ServerDescription::operator=(ServerDescription&& other) noexcept -{ - if (this != &other) - { - _address = std::move(other._address); - _type = other._type; - _lastUpdateTime = other._lastUpdateTime; - _roundTripTime = other._roundTripTime; - _setName = std::move(other._setName); - _hosts = std::move(other._hosts); - _tags = std::move(other._tags); - _error = std::move(other._error); - _hasError = other._hasError; - - other._type = Unknown; - other._roundTripTime = 0; - other._hasError = false; - } - return *this; -} +ServerDescription& ServerDescription::operator=(ServerDescription&& other) noexcept = default; void ServerDescription::updateFromHelloResponse(const Document& helloResponse, Poco::Int64 rttMicros) @@ -220,12 +140,13 @@ void ServerDescription::parseHosts(const Document& doc) if (doc.exists("hosts")) { Array::Ptr hostsArray = doc.get("hosts"); - for (int i = 0; i < hostsArray->size(); ++i) + _hosts.reserve(hostsArray->size()); + for (std::size_t i = 0; i < hostsArray->size(); ++i) { try { std::string hostStr = hostsArray->get(i); - _hosts.push_back(Net::SocketAddress(hostStr)); + _hosts.emplace_back(hostStr); } catch (...) { @@ -238,12 +159,12 @@ void ServerDescription::parseHosts(const Document& doc) if (doc.exists("passives")) { Array::Ptr passivesArray = doc.get("passives"); - for (int i = 0; i < passivesArray->size(); ++i) + for (std::size_t i = 0; i < passivesArray->size(); ++i) { try { std::string hostStr = passivesArray->get(i); - _hosts.push_back(Net::SocketAddress(hostStr)); + _hosts.emplace_back(hostStr); } catch (...) { @@ -256,12 +177,12 @@ void ServerDescription::parseHosts(const Document& doc) if (doc.exists("arbiters")) { Array::Ptr arbitersArray = doc.get("arbiters"); - for (int i = 0; i < arbitersArray->size(); ++i) + for (std::size_t i = 0; i < arbitersArray->size(); ++i) { try { std::string hostStr = arbitersArray->get(i); - _hosts.push_back(Net::SocketAddress(hostStr)); + _hosts.emplace_back(hostStr); } catch (...) { diff --git a/MongoDB/src/TopologyDescription.cpp b/MongoDB/src/TopologyDescription.cpp index fb8fe6515..bac45056d 100644 --- a/MongoDB/src/TopologyDescription.cpp +++ b/MongoDB/src/TopologyDescription.cpp @@ -20,20 +20,11 @@ namespace Poco { namespace MongoDB { -TopologyDescription::TopologyDescription(): - _mutex(), - _type(Unknown), - _setName(), - _servers() -{ -} +TopologyDescription::TopologyDescription() = default; TopologyDescription::TopologyDescription(const std::string& setName): - _mutex(), - _type(Unknown), - _setName(setName), - _servers() + _setName(setName) { } @@ -53,13 +44,10 @@ TopologyDescription::TopologyDescription(TopologyDescription&& other) noexcept _type = other._type; _setName = std::move(other._setName); _servers = std::move(other._servers); - other._type = Unknown; } -TopologyDescription::~TopologyDescription() -{ -} +TopologyDescription::~TopologyDescription() = default; TopologyDescription& TopologyDescription::operator=(const TopologyDescription& other) @@ -89,7 +77,6 @@ TopologyDescription& TopologyDescription::operator=(TopologyDescription&& other) _type = other._type; _setName = std::move(other._setName); _servers = std::move(other._servers); - other._type = Unknown; } return *this; } @@ -121,9 +108,9 @@ std::vector TopologyDescription::servers() const Mutex::ScopedLock lock(_mutex); std::vector result; result.reserve(_servers.size()); - for (const auto& pair : _servers) + for (const auto& [address, server] : _servers) { - result.push_back(pair.second); + result.emplace_back(server); } return result; } @@ -132,11 +119,11 @@ std::vector TopologyDescription::servers() const ServerDescription TopologyDescription::findPrimary() const { Mutex::ScopedLock lock(_mutex); - for (const auto& pair : _servers) + for (const auto& [address, server] : _servers) { - if (pair.second.isPrimary()) + if (server.isPrimary()) { - return pair.second; + return server; } } return ServerDescription(); @@ -147,11 +134,12 @@ std::vector TopologyDescription::findSecondaries() const { Mutex::ScopedLock lock(_mutex); std::vector result; - for (const auto& pair : _servers) + result.reserve(_servers.size()); + for (const auto& [address, server] : _servers) { - if (pair.second.isSecondary()) + if (server.isSecondary()) { - result.push_back(pair.second); + result.emplace_back(server); } } return result; @@ -161,9 +149,9 @@ std::vector TopologyDescription::findSecondaries() const bool TopologyDescription::hasPrimary() const { Mutex::ScopedLock lock(_mutex); - for (const auto& pair : _servers) + for (const auto& [address, server] : _servers) { - if (pair.second.isPrimary()) + if (server.isPrimary()) { return true; } @@ -199,7 +187,7 @@ void TopologyDescription::updateServer(const Net::SocketAddress& address, const auto it = _servers.find(address); if (it == _servers.end()) { - it = _servers.insert(std::make_pair(address, ServerDescription(address))).first; + it = _servers.try_emplace(address, address).first; } // Update from hello response @@ -236,9 +224,9 @@ void TopologyDescription::addServer(const Net::SocketAddress& address) { Mutex::ScopedLock lock(_mutex); - if (_servers.find(address) == _servers.end()) + auto [it, inserted] = _servers.try_emplace(address, address); + if (inserted) { - _servers.insert(std::make_pair(address, ServerDescription(address))); updateTopologyType(); } } @@ -286,9 +274,9 @@ void TopologyDescription::updateTopologyType() int unknownCount = 0; int standaloneCount = 0; - for (const auto& pair : _servers) + for (const auto& [address, server] : _servers) { - switch (pair.second.type()) + switch (server.type()) { case ServerDescription::RsPrimary: primaries++; @@ -340,13 +328,10 @@ void TopologyDescription::processNewHosts(const ServerDescription& serverDesc) // This method must be called while holding the mutex // Add newly discovered hosts to the topology - const std::vector& hosts = serverDesc.hosts(); + const auto& hosts = serverDesc.hosts(); for (const auto& host : hosts) { - if (_servers.find(host) == _servers.end()) - { - _servers.insert(std::make_pair(host, ServerDescription(host))); - } + _servers.try_emplace(host, host); } }