From 7b40c5f2cb5a38b93d8471534244dc3f7147676c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20B=C3=A9raud?= <adrien.beraud@savoirfairelinux.com> Date: Tue, 25 Mar 2025 11:52:11 -0400 Subject: [PATCH] json: use toString Use a single static instance of Json::StreamWriterBuilder instead of creating a new one repeatedly. Move json-related symbols to their own namespace. Change-Id: I5145a71d7b19fd2aac158b51774e964ce772311b --- src/call.cpp | 10 +--- src/conference.cpp | 4 +- src/jamidht/account_manager.cpp | 2 +- src/jamidht/accountarchive.cpp | 7 +-- src/jamidht/conversation.cpp | 34 +++++-------- src/jamidht/conversation_module.cpp | 4 +- src/jamidht/conversationrepository.cpp | 51 +++++-------------- src/jamidht/jamiaccount.cpp | 10 ++-- src/jamidht/namedirectory.cpp | 4 +- src/json_utils.h | 11 +++- src/string_utils.cpp | 11 ++++ test/unitTest/conversation/conversation.cpp | 45 +++++++--------- .../conversation/conversationcommon.cpp | 17 ++----- 13 files changed, 85 insertions(+), 125 deletions(-) diff --git a/src/call.cpp b/src/call.cpp index 2eb52f1ad..dc3e77c1b 100644 --- a/src/call.cpp +++ b/src/call.cpp @@ -637,7 +637,7 @@ Call::setConferenceInfo(const std::string& msg) { ConfInfo newInfo; Json::Value json; - if (parseJson(msg, json)) { + if (json::parse(msg, json)) { if (json.isObject()) { // new confInfo if (json.isMember("p")) { @@ -692,10 +692,7 @@ void Call::sendConfOrder(const Json::Value& root) { std::map<std::string, std::string> messages; - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; - messages["application/confOrder+json"] = Json::writeString(wbuilder, root); + messages["application/confOrder+json"] = json::toString(root); auto w = getAccount(); auto account = w.lock(); @@ -707,9 +704,6 @@ void Call::sendConfInfo(const std::string& json) { std::map<std::string, std::string> messages; - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; messages["application/confInfo+json"] = json; auto w = getAccount(); diff --git a/src/conference.cpp b/src/conference.cpp index 47cfb7e6c..0cae344d2 100644 --- a/src/conference.cpp +++ b/src/conference.cpp @@ -855,7 +855,7 @@ ConfInfo::toString() const val["h"] = h; val["v"] = v; val["layout"] = layout; - return Json::writeString(Json::StreamWriterBuilder {}, val); + return json::toString(val); } void @@ -1120,7 +1120,7 @@ Conference::onConfOrder(const std::string& callId, const std::string& confOrder) if (auto call = getCall(callId)) { const auto& peerId = getRemoteId(call); Json::Value root; - if (!parseJson(confOrder, root)) { + if (!json::parse(confOrder, root)) { JAMI_WARNING("Unable to parse conference order from {}", peerId); return; } diff --git a/src/jamidht/account_manager.cpp b/src/jamidht/account_manager.cpp index 6f839a1bc..6088c4e84 100644 --- a/src/jamidht/account_manager.cpp +++ b/src/jamidht/account_manager.cpp @@ -215,7 +215,7 @@ AccountManager::useIdentity(const dht::crypto::Identity& identity, } Json::Value root; - if (!parseJson(receipt, root) || !root.isMember("announce")) { + if (!json::parse(receipt, root) || !root.isMember("announce")) { JAMI_ERROR("[Account {}] [Auth] device receipt parsing error", accountId_); return nullptr; } diff --git a/src/jamidht/accountarchive.cpp b/src/jamidht/accountarchive.cpp index b9a28a127..3585185c7 100644 --- a/src/jamidht/accountarchive.cpp +++ b/src/jamidht/accountarchive.cpp @@ -21,7 +21,7 @@ #include "base64.h" #include "logger.h" -#include <json/json.h> +#include <json_utils.h> namespace jami { @@ -147,10 +147,7 @@ AccountArchive::serialize() const } } - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; - return Json::writeString(wbuilder, root); + return json::toString(root); } } // namespace jami diff --git a/src/jamidht/conversation.cpp b/src/jamidht/conversation.cpp index 495ca4fbd..a44d719f2 100644 --- a/src/jamidht/conversation.cpp +++ b/src/jamidht/conversation.cpp @@ -17,16 +17,8 @@ #include "conversation.h" #include "account_const.h" -#include "fileutils.h" #include "jamiaccount.h" #include "client/ring_signal.h" - -#include <charconv> -#include <json/json.h> -#include <string_view> -#include <opendht/thread_pool.h> -#include <tuple> -#include <optional> #include "swarm/swarm_manager.h" #ifdef ENABLE_PLUGIN #include "manager.h" @@ -35,15 +27,19 @@ #endif #include "jami/conversation_interface.h" +#include "fileutils.h" +#include "json_utils.h" + +#include <opendht/thread_pool.h> + +#include <charconv> +#include <string_view> +#include <tuple> +#include <optional> + namespace jami { static const char* const LAST_MODIFIED = "lastModified"; -static const auto jsonBuilder = [] { - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; - return wbuilder; -}(); ConvInfo::ConvInfo(const Json::Value& json) { @@ -806,7 +802,7 @@ Conversation::Impl::commitsEndedCalls() }); if (itActive != activeCalls_.end()) activeCalls_.erase(itActive); - commits.emplace_back(repository_->commitMessage(Json::writeString(jsonBuilder, value))); + commits.emplace_back(repository_->commitMessage(json::toString(value))); JAMI_DEBUG("Removing hosted conference... {:s}", hostedCall.first); } @@ -1640,8 +1636,7 @@ Conversation::sendMessage(Json::Value&& value, [w = weak(), value = std::move(value), onCommit = std::move(onCommit), cb = std::move(cb)] { if (auto sthis = w.lock()) { std::unique_lock lk(sthis->pimpl_->writeMtx_); - auto commit = sthis->pimpl_->repository_->commitMessage( - Json::writeString(jsonBuilder, value)); + auto commit = sthis->pimpl_->repository_->commitMessage(json::toString(value)); lk.unlock(); if (onCommit) onCommit(commit); @@ -1661,8 +1656,7 @@ Conversation::sendMessages(std::vector<Json::Value>&& messages, OnMultiDoneCb&& commits.reserve(messages.size()); std::unique_lock lk(sthis->pimpl_->writeMtx_); for (const auto& message : messages) { - auto commit = sthis->pimpl_->repository_->commitMessage( - Json::writeString(jsonBuilder, message)); + auto commit = sthis->pimpl_->repository_->commitMessage(json::toString(message)); commits.emplace_back(std::move(commit)); } lk.unlock(); @@ -1951,7 +1945,7 @@ Conversation::generateInvitation() const metadata[k] = v; } root[ConversationMapKeys::CONVERSATIONID] = id(); - return {{"application/invite+json", Json::writeString(jsonBuilder, root)}}; + return {{"application/invite+json", json::toString(root)}}; } std::string diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp index 71b2e5d68..be8d11b45 100644 --- a/src/jamidht/conversation_module.cpp +++ b/src/jamidht/conversation_module.cpp @@ -31,6 +31,7 @@ #include "manager.h" #include "sip/sipcall.h" #include "vcard.h" +#include "json_utils.h" namespace jami { @@ -1089,8 +1090,7 @@ ConversationModule::Impl::sendMessageNotification(Conversation& conversation, message["id"] = conversation.id(); message["commit"] = commit; message["deviceId"] = deviceId_; - Json::StreamWriterBuilder builder; - const auto text = Json::writeString(builder, message); + const auto text = json::toString(message); // Send message notification will announce the new commit in 3 steps. diff --git a/src/jamidht/conversationrepository.cpp b/src/jamidht/conversationrepository.cpp index 0bdafb080..495a0db26 100644 --- a/src/jamidht/conversationrepository.cpp +++ b/src/jamidht/conversationrepository.cpp @@ -688,9 +688,6 @@ initial_commit(GitRepository& repo, json["invited"] = otherMember; } json["type"] = "initial"; - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; git_buf to_sign = {}; if (git_commit_create_buffer(&to_sign, @@ -698,7 +695,7 @@ initial_commit(GitRepository& repo, sig.get(), sig.get(), nullptr, - Json::writeString(wbuilder, json).c_str(), + json::toString(json).c_str(), tree.get(), 0, nullptr) @@ -1742,7 +1739,7 @@ ConversationRepository::Impl::checkInitialCommit(const std::string& userDevice, std::string invited = {}; if (mode_ == ConversationMode::ONE_TO_ONE) { Json::Value cm; - if (parseJson(commitMsg, cm)) { + if (json::parse(commitMsg, cm)) { invited = cm["invited"].asString(); } } @@ -2009,7 +2006,7 @@ ConversationRepository::Impl::mode() const auto commitMsg = lastMsg[0].commit_msg; Json::Value root; - if (!parseJson(commitMsg, root)) { + if (!json::parse(commitMsg, root)) { emitSignal<libjami::ConversationSignal::OnConversationError>(accountId_, id_, EINVALIDMODE, @@ -2378,7 +2375,7 @@ ConversationRepository::Impl::getInitialMembers() const auto authorId = cert->issuer->getId().toString(); if (mode() == ConversationMode::ONE_TO_ONE) { Json::Value root; - if (!parseJson(commit.commit_msg, root)) { + if (!json::parse(commit.commit_msg, root)) { return {authorId}; } if (root.isMember("invited") && root["invited"].asString() != authorId) @@ -2519,7 +2516,7 @@ ConversationRepository::Impl::convCommitToMap(const ConversationCommit& commit) std::map<std::string, std::string> message; if (type.empty()) { Json::Value cm; - if (parseJson(commit.commit_msg, cm)) { + if (json::parse(commit.commit_msg, cm)) { for (auto const& id : cm.getMemberNames()) { if (id == "type") { type = cm[id].asString(); @@ -2712,7 +2709,7 @@ ConversationRepository::Impl::validCommits( } else if (commit.parents.size() == 1) { std::string type = {}, editId = {}; Json::Value cm; - if (parseJson(commit.commit_msg, cm)) { + if (json::parse(commit.commit_msg, cm)) { type = cm["type"].asString(); editId = cm["edit"].asString(); } else { @@ -2939,10 +2936,7 @@ ConversationRepository::addMember(const std::string& uri) json["action"] = "add"; json["uri"] = uri; json["type"] = "member"; - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; - return pimpl_->commit(Json::writeString(wbuilder, json)); + return pimpl_->commit(json::toString(json)); } void @@ -3405,9 +3399,6 @@ ConversationRepository::join() json["action"] = "join"; json["uri"] = uri; json["type"] = "member"; - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; { std::lock_guard lk(pimpl_->membersMtx_); @@ -3425,7 +3416,7 @@ ConversationRepository::join() pimpl_->saveMembers(); } - return pimpl_->commitMessage(Json::writeString(wbuilder, json)); + return pimpl_->commitMessage(json::toString(json)); } std::string @@ -3486,9 +3477,6 @@ ConversationRepository::leave() json["action"] = "remove"; json["uri"] = pimpl_->userId_; json["type"] = "member"; - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; { std::lock_guard lk(pimpl_->membersMtx_); @@ -3498,7 +3486,7 @@ ConversationRepository::leave() pimpl_->saveMembers(); } - return pimpl_->commit(Json::writeString(wbuilder, json), false); + return pimpl_->commit(json::toString(json), false); } void @@ -3564,10 +3552,7 @@ ConversationRepository::voteKick(const std::string& uri, const std::string& type Json::Value json; json["uri"] = uri; json["type"] = "vote"; - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; - return pimpl_->commitMessage(Json::writeString(wbuilder, json)); + return pimpl_->commitMessage(json::toString(json)); } std::string @@ -3606,10 +3591,7 @@ ConversationRepository::voteUnban(const std::string& uri, const std::string_view Json::Value json; json["uri"] = uri; json["type"] = "vote"; - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; - return pimpl_->commitMessage(Json::writeString(wbuilder, json)); + return pimpl_->commitMessage(json::toString(json)); } bool @@ -3760,10 +3742,7 @@ ConversationRepository::resolveVote(const std::string& uri, json["action"] = voteType; json["uri"] = uri; json["type"] = "member"; - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; - return pimpl_->commitMessage(Json::writeString(wbuilder, json)); + return pimpl_->commitMessage(json::toString(json)); } // If vote nok @@ -3954,11 +3933,7 @@ ConversationRepository::updateInfos(const std::map<std::string, std::string>& pr return {}; Json::Value json; json["type"] = "application/update-profile"; - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; - - return pimpl_->commitMessage(Json::writeString(wbuilder, json)); + return pimpl_->commitMessage(json::toString(json)); } std::map<std::string, std::string> diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp index 6d8ed25cc..e92e1585a 100644 --- a/src/jamidht/jamiaccount.cpp +++ b/src/jamidht/jamiaccount.cpp @@ -928,7 +928,7 @@ JamiAccount::loadConfig() auto str = fileutils::loadCacheTextFile(cachePath_ / "dhtproxy", std::chrono::hours(24 * 7)); Json::Value root; - if (parseJson(str, root)) { + if (json::parse(str, root)) { proxyServerCached_ = root[getProxyConfigKey()].asString(); } } catch (const std::exception& e) { @@ -2068,7 +2068,7 @@ JamiAccount::doRegister_() }); connectionManager_->onChannelRequest( [this](const std::shared_ptr<dht::crypto::Certificate>& cert, const std::string& name) { - JAMI_WARNING("[Account {}] New channel asked with name {} from {}", + JAMI_LOG("[Account {}] New channel requested with name {} from {}", getAccountID(), name, cert->issuer->getId()); @@ -3213,7 +3213,7 @@ JamiAccount::sendMessage(const std::string& to, // to load the conversation. auto extractIdFromJson = [](const std::string& jsonData) -> std::string { Json::Value parsed; - if (parseJson(jsonData, parsed)) { + if (json::parse(jsonData, parsed)) { auto value = parsed.get("id", Json::nullValue); if (value && value.isString()) { return value.asString(); @@ -3568,7 +3568,7 @@ JamiAccount::handleMessage(const std::string& from, const std::pair<std::string, { if (m.first == MIME_TYPE_GIT) { Json::Value json; - if (!parseJson(m.second, json)) { + if (!json::parse(m.second, json)) { return false; } @@ -3588,7 +3588,7 @@ JamiAccount::handleMessage(const std::string& from, const std::pair<std::string, return true; } else if (m.first == MIME_TYPE_INVITE_JSON) { Json::Value json; - if (!parseJson(m.second, json)) { + if (!json::parse(m.second, json)) { return false; } convModule()->onConversationRequest(from, json); diff --git a/src/jamidht/namedirectory.cpp b/src/jamidht/namedirectory.cpp index 38716acc3..da4d2c62a 100644 --- a/src/jamidht/namedirectory.cpp +++ b/src/jamidht/namedirectory.cpp @@ -200,7 +200,7 @@ NameDirectory::lookupAddress(const std::string& addr, LookupCallback cb) } else { try { Json::Value json; - if (!parseJson(response.body, json)) { + if (!json::parse(response.body, json)) { cb("", "", Response::error); return; } @@ -276,7 +276,7 @@ NameDirectory::lookupName(const std::string& name, LookupCallback cb) } else { try { Json::Value json; - if (!parseJson(response.body, json)) { + if (!json::parse(response.body, json)) { cb("", "", Response::error); return; } diff --git a/src/json_utils.h b/src/json_utils.h index c70fad5ce..50addef60 100644 --- a/src/json_utils.h +++ b/src/json_utils.h @@ -21,15 +21,17 @@ #include <logger.h> namespace jami { +namespace json { extern const Json::CharReaderBuilder rbuilder; +extern const Json::StreamWriterBuilder wbuilder; -inline bool parseJson(std::string_view jsonStr, Json::Value& json) { +inline bool parse(std::string_view jsonStr, Json::Value& jsonVal) { std::string err; auto reader = std::unique_ptr<Json::CharReader>(rbuilder.newCharReader()); if (!reader->parse(jsonStr.data(), jsonStr.data() + jsonStr.size(), - &json, + &jsonVal, &err)) { JAMI_WARNING("Can't parse JSON: {}\n{}", err, jsonStr); return false; @@ -37,4 +39,9 @@ inline bool parseJson(std::string_view jsonStr, Json::Value& json) { return true; } +inline std::string toString(const Json::Value& jsonVal) { + return Json::writeString(wbuilder, jsonVal); +} + +} } diff --git a/src/string_utils.cpp b/src/string_utils.cpp index e1830e263..db83028dc 100644 --- a/src/string_utils.cpp +++ b/src/string_utils.cpp @@ -43,6 +43,7 @@ namespace jami { +namespace json { Json::CharReaderBuilder getJsonReaderBuilder() { Json::CharReaderBuilder builder; @@ -50,7 +51,17 @@ Json::CharReaderBuilder getJsonReaderBuilder() return builder; } +Json::StreamWriterBuilder getJsonWriterBuilder() +{ + Json::StreamWriterBuilder builder; + builder["commentStyle"] = "None"; + builder["indentation"] = ""; + return builder; +} + const Json::CharReaderBuilder rbuilder = getJsonReaderBuilder(); +const Json::StreamWriterBuilder wbuilder = getJsonWriterBuilder(); +} std::string_view userAgent() diff --git a/test/unitTest/conversation/conversation.cpp b/test/unitTest/conversation/conversation.cpp index 94db88dad..96b7e6272 100644 --- a/test/unitTest/conversation/conversation.cpp +++ b/test/unitTest/conversation/conversation.cpp @@ -19,24 +19,27 @@ #include <cppunit/TestFixture.h> #include <cppunit/extensions/HelperMacros.h> -#include <condition_variable> -#include <string> -#include <fstream> -#include <streambuf> -#include <git2.h> -#include <filesystem> -#include <msgpack.hpp> - #include "../../test_runner.h" +#include "common.h" +#include "conversation/conversationcommon.h" + +#include "jami.h" #include "account_const.h" #include "archiver.h" #include "base64.h" -#include "common.h" -#include "conversation/conversationcommon.h" #include "fileutils.h" -#include "jami.h" +#include "json_utils.h" #include "manager.h" + #include <dhtnet/certstore.h> +#include <msgpack.hpp> +#include <git2.h> + +#include <condition_variable> +#include <string> +#include <fstream> +#include <streambuf> +#include <filesystem> using namespace std::string_literals; using namespace std::literals::chrono_literals; @@ -747,10 +750,7 @@ ConversationTest::testReplaceWithBadCertificate() Json::Value root; root["type"] = "text/plain"; root["body"] = "hi"; - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; - auto message = Json::writeString(wbuilder, root); + auto message = json::toString(root); commitInRepo(repoPath, aliceAccount, message); // now we need to sync! bobData.errorDetected = false; @@ -1242,16 +1242,13 @@ ConversationTest::createFakeConversation(std::shared_ptr<JamiAccount> account, Json::Value json; json["mode"] = 1; json["type"] = "initial"; - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; if (git_commit_create_buffer(&to_sign, repo.get(), sig.get(), sig.get(), nullptr, - Json::writeString(wbuilder, json).c_str(), + json::toString(json).c_str(), tree.get(), 0, nullptr) @@ -1560,10 +1557,7 @@ ConversationTest::testUnknownModeDetected() Json::Value json; json["mode"] = 1412; json["type"] = "initial"; - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; - repo.amend(convId, Json::writeString(wbuilder, json)); + repo.amend(convId, json::toString(json)); libjami::addConversationMember(aliceId, convId, bobUri); CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return bobData.requestReceived; })); libjami::acceptConversationRequest(bobId, convId); @@ -2315,11 +2309,8 @@ ConversationTest::testMessageEdition() root["type"] = "text/plain"; root["edit"] = convId; root["body"] = "new"; - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; auto repoPath = fileutils::get_data_dir() / aliceId / "conversations" / convId; - auto message = Json::writeString(wbuilder, root); + auto message = json::toString(root); commitInRepo(repoPath, aliceAccount, message); bobData.errorDetected = false; libjami::sendMessage(aliceId, convId, "trigger"s, ""); diff --git a/test/unitTest/conversation/conversationcommon.cpp b/test/unitTest/conversation/conversationcommon.cpp index 139d2a5cd..005ec3be0 100644 --- a/test/unitTest/conversation/conversationcommon.cpp +++ b/test/unitTest/conversation/conversationcommon.cpp @@ -33,6 +33,7 @@ #include "jamidht/conversation.h" #include "jamidht/conversationrepository.h" #include "conversation/conversationcommon.h" +#include "json_utils.h" using namespace std::string_literals; @@ -66,11 +67,8 @@ addVote(std::shared_ptr<JamiAccount> account, Json::Value json; json["uri"] = votedUri; json["type"] = "vote"; - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; ConversationRepository cr(account, convId); - cr.commitMessage(Json::writeString(wbuilder, json), false); + cr.commitMessage(json::toString(json), false); } void @@ -107,10 +105,7 @@ simulateRemoval(std::shared_ptr<JamiAccount> account, json["action"] = "ban"; json["uri"] = votedUri; json["type"] = "member"; - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; - cr.commitMessage(Json::writeString(wbuilder, json)); + cr.commitMessage(json::toString(json)); libjami::sendMessage(account->getAccountID(), convId, @@ -180,11 +175,7 @@ commit(std::shared_ptr<JamiAccount> account, const std::string& convId, Json::Va { ConversationRepository::DISABLE_RESET = true; ConversationRepository cr(account, convId); - - Json::StreamWriterBuilder wbuilder; - wbuilder["commentStyle"] = "None"; - wbuilder["indentation"] = ""; - cr.commitMessage(Json::writeString(wbuilder, message)); + cr.commitMessage(json::toString(message)); } std::string -- GitLab