From a34e5e51d8c0e2a0c8b6224db806dfe20d0319ce Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 10 Oct 2018 20:57:29 -0400 Subject: telemetry_json: Remove unnecessary includes Removes unused includes. Also rectifies a missing include. --- src/web_service/telemetry_json.cpp | 2 -- 1 file changed, 2 deletions(-) (limited to 'src/web_service/telemetry_json.cpp') diff --git a/src/web_service/telemetry_json.cpp b/src/web_service/telemetry_json.cpp index 033ea1ea4..e77950aa3 100644 --- a/src/web_service/telemetry_json.cpp +++ b/src/web_service/telemetry_json.cpp @@ -2,8 +2,6 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. -#include -#include "common/assert.h" #include "common/detached_tasks.h" #include "web_service/telemetry_json.h" #include "web_service/web_backend.h" -- cgit v1.2.3 From 881bb2295d40c50a401cd05fff8eeb822ec577e9 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 10 Oct 2018 20:59:25 -0400 Subject: telemetry_json: Take std::string parameters by value Taking them by const reference isn't advisable here, because it means the std::move calls were doing nothing and we were always copying the std::string instances. --- src/web_service/telemetry_json.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'src/web_service/telemetry_json.cpp') diff --git a/src/web_service/telemetry_json.cpp b/src/web_service/telemetry_json.cpp index e77950aa3..b24e806a8 100644 --- a/src/web_service/telemetry_json.cpp +++ b/src/web_service/telemetry_json.cpp @@ -8,8 +8,7 @@ namespace WebService { -TelemetryJson::TelemetryJson(const std::string& host, const std::string& username, - const std::string& token) +TelemetryJson::TelemetryJson(std::string host, std::string username, std::string token) : host(std::move(host)), username(std::move(username)), token(std::move(token)) {} TelemetryJson::~TelemetryJson() = default; -- cgit v1.2.3 From a7725d354cf118b2299ed197d4b88ff48ebc1341 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 10 Oct 2018 21:04:19 -0400 Subject: telemetry_json: Use the PImpl idiom to avoid unnecessary dependency exposure Users of the web_service library shouldn't need to care about an external library like json.h. However, given it's exposed in our interface, this requires that other libraries publicly link in the JSON library. We can do better. By using the PImpl idiom, we can hide this dependency in the cpp file and remove the need to link that library in altogether. --- src/web_service/telemetry_json.cpp | 86 +++++++++++++++++++++++--------------- 1 file changed, 53 insertions(+), 33 deletions(-) (limited to 'src/web_service/telemetry_json.cpp') diff --git a/src/web_service/telemetry_json.cpp b/src/web_service/telemetry_json.cpp index b24e806a8..3c5590054 100644 --- a/src/web_service/telemetry_json.cpp +++ b/src/web_service/telemetry_json.cpp @@ -2,93 +2,113 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. +#include #include "common/detached_tasks.h" #include "web_service/telemetry_json.h" #include "web_service/web_backend.h" namespace WebService { -TelemetryJson::TelemetryJson(std::string host, std::string username, std::string token) - : host(std::move(host)), username(std::move(username)), token(std::move(token)) {} -TelemetryJson::~TelemetryJson() = default; +struct TelemetryJson::Impl { + Impl(std::string host, std::string username, std::string token) + : host{std::move(host)}, username{std::move(username)}, token{std::move(token)} {} -template -void TelemetryJson::Serialize(Telemetry::FieldType type, const std::string& name, T value) { - sections[static_cast(type)][name] = value; -} + nlohmann::json& TopSection() { + return sections[static_cast(Telemetry::FieldType::None)]; + } -void TelemetryJson::SerializeSection(Telemetry::FieldType type, const std::string& name) { - TopSection()[name] = sections[static_cast(type)]; -} + const nlohmann::json& TopSection() const { + return sections[static_cast(Telemetry::FieldType::None)]; + } + + template + void Serialize(Telemetry::FieldType type, const std::string& name, T value) { + sections[static_cast(type)][name] = value; + } + + void SerializeSection(Telemetry::FieldType type, const std::string& name) { + TopSection()[name] = sections[static_cast(type)]; + } + + nlohmann::json output; + std::array sections; + std::string host; + std::string username; + std::string token; +}; + +TelemetryJson::TelemetryJson(std::string host, std::string username, std::string token) + : impl{std::make_unique(std::move(host), std::move(username), std::move(token))} {} +TelemetryJson::~TelemetryJson() = default; void TelemetryJson::Visit(const Telemetry::Field& field) { - Serialize(field.GetType(), field.GetName(), field.GetValue()); + impl->Serialize(field.GetType(), field.GetName(), field.GetValue()); } void TelemetryJson::Visit(const Telemetry::Field& field) { - Serialize(field.GetType(), field.GetName(), field.GetValue()); + impl->Serialize(field.GetType(), field.GetName(), field.GetValue()); } void TelemetryJson::Visit(const Telemetry::Field& field) { - Serialize(field.GetType(), field.GetName(), field.GetValue()); + impl->Serialize(field.GetType(), field.GetName(), field.GetValue()); } void TelemetryJson::Visit(const Telemetry::Field& field) { - Serialize(field.GetType(), field.GetName(), field.GetValue()); + impl->Serialize(field.GetType(), field.GetName(), field.GetValue()); } void TelemetryJson::Visit(const Telemetry::Field& field) { - Serialize(field.GetType(), field.GetName(), field.GetValue()); + impl->Serialize(field.GetType(), field.GetName(), field.GetValue()); } void TelemetryJson::Visit(const Telemetry::Field& field) { - Serialize(field.GetType(), field.GetName(), field.GetValue()); + impl->Serialize(field.GetType(), field.GetName(), field.GetValue()); } void TelemetryJson::Visit(const Telemetry::Field& field) { - Serialize(field.GetType(), field.GetName(), field.GetValue()); + impl->Serialize(field.GetType(), field.GetName(), field.GetValue()); } void TelemetryJson::Visit(const Telemetry::Field& field) { - Serialize(field.GetType(), field.GetName(), field.GetValue()); + impl->Serialize(field.GetType(), field.GetName(), field.GetValue()); } void TelemetryJson::Visit(const Telemetry::Field& field) { - Serialize(field.GetType(), field.GetName(), field.GetValue()); + impl->Serialize(field.GetType(), field.GetName(), field.GetValue()); } void TelemetryJson::Visit(const Telemetry::Field& field) { - Serialize(field.GetType(), field.GetName(), field.GetValue()); + impl->Serialize(field.GetType(), field.GetName(), field.GetValue()); } void TelemetryJson::Visit(const Telemetry::Field& field) { - Serialize(field.GetType(), field.GetName(), field.GetValue()); + impl->Serialize(field.GetType(), field.GetName(), field.GetValue()); } void TelemetryJson::Visit(const Telemetry::Field& field) { - Serialize(field.GetType(), field.GetName(), field.GetValue()); + impl->Serialize(field.GetType(), field.GetName(), field.GetValue()); } void TelemetryJson::Visit(const Telemetry::Field& field) { - Serialize(field.GetType(), field.GetName(), std::string(field.GetValue())); + impl->Serialize(field.GetType(), field.GetName(), std::string(field.GetValue())); } void TelemetryJson::Visit(const Telemetry::Field& field) { - Serialize(field.GetType(), field.GetName(), field.GetValue().count()); + impl->Serialize(field.GetType(), field.GetName(), field.GetValue().count()); } void TelemetryJson::Complete() { - SerializeSection(Telemetry::FieldType::App, "App"); - SerializeSection(Telemetry::FieldType::Session, "Session"); - SerializeSection(Telemetry::FieldType::Performance, "Performance"); - SerializeSection(Telemetry::FieldType::UserFeedback, "UserFeedback"); - SerializeSection(Telemetry::FieldType::UserConfig, "UserConfig"); - SerializeSection(Telemetry::FieldType::UserSystem, "UserSystem"); - - auto content = TopSection().dump(); + impl->SerializeSection(Telemetry::FieldType::App, "App"); + impl->SerializeSection(Telemetry::FieldType::Session, "Session"); + impl->SerializeSection(Telemetry::FieldType::Performance, "Performance"); + impl->SerializeSection(Telemetry::FieldType::UserFeedback, "UserFeedback"); + impl->SerializeSection(Telemetry::FieldType::UserConfig, "UserConfig"); + impl->SerializeSection(Telemetry::FieldType::UserSystem, "UserSystem"); + + auto content = impl->TopSection().dump(); // Send the telemetry async but don't handle the errors since they were written to the log Common::DetachedTasks::AddTask( - [host{this->host}, username{this->username}, token{this->token}, content]() { + [host{impl->host}, username{impl->username}, token{impl->token}, content]() { Client{host, username, token}.PostJson("/telemetry", content, true); }); } -- cgit v1.2.3 From 183a664405661fb531b82e2a0a3a8c6651a1ce8a Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 10 Oct 2018 21:23:41 -0400 Subject: web_backend: Make Client use the PImpl idiom Like with TelemetryJson, we can make the implementation details private and avoid the need to expose httplib to external libraries that need to use the Client class. --- src/web_service/telemetry_json.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'src/web_service/telemetry_json.cpp') diff --git a/src/web_service/telemetry_json.cpp b/src/web_service/telemetry_json.cpp index 3c5590054..0a8f2bd9e 100644 --- a/src/web_service/telemetry_json.cpp +++ b/src/web_service/telemetry_json.cpp @@ -4,6 +4,7 @@ #include #include "common/detached_tasks.h" +#include "common/web_result.h" #include "web_service/telemetry_json.h" #include "web_service/web_backend.h" -- cgit v1.2.3