From c65b7b3e0b4725f9405321cd4214b59e86659759 Mon Sep 17 00:00:00 2001 From: emeric Date: Fri, 19 Sep 2025 00:34:37 +0200 Subject: [PATCH] replaced boost by manual xml writing, to improve serialization perfs --- src/libs/core/impl/String.cpp | 22 +- src/libs/core/include/core/String.hpp | 3 + src/libs/core/test/String.cpp | 21 ++ src/libs/subsonic/CMakeLists.txt | 4 + src/libs/subsonic/impl/SubsonicResponse.cpp | 282 +++++++++--------- src/libs/subsonic/impl/SubsonicResponse.hpp | 17 +- src/libs/subsonic/test/CMakeLists.txt | 20 ++ .../subsonic/test/SubsonicResponseTest.cpp | 107 +++++++ 8 files changed, 335 insertions(+), 141 deletions(-) create mode 100644 src/libs/subsonic/test/CMakeLists.txt create mode 100644 src/libs/subsonic/test/SubsonicResponseTest.cpp diff --git a/src/libs/core/impl/String.cpp b/src/libs/core/impl/String.cpp index 64650c77..31a1e84b 100644 --- a/src/libs/core/impl/String.cpp +++ b/src/libs/core/impl/String.cpp @@ -44,10 +44,20 @@ namespace lms::core::stringUtils constexpr std::pair jsonEscapeChars[]{ { '\\', "\\\\" }, + { '"', "\\\"" }, + { '\b', "\\b" }, + { '\f', "\\f" }, { '\n', "\\n" }, { '\r', "\\r" }, { '\t', "\\t" }, - { '"', "\\\"" }, + }; + + constexpr std::pair xmlEscapeChars[]{ + { '&', "&" }, + { '<', "<" }, + { '>', ">" }, + { '\'', "'" }, + { '"', """ }, }; template @@ -442,6 +452,16 @@ namespace lms::core::stringUtils details::writeEscapedString(os, str, details::jsonEscapeChars); } + std::string xmlEscape(std::string_view str) + { + return details::escape(str, details::xmlEscapeChars); + } + + void writeXmlEscapedString(std::ostream& os, std::string_view str) + { + details::writeEscapedString(os, str, details::xmlEscapeChars); + } + std::string escapeString(std::string_view str, std::string_view charsToEscape, char escapeChar) { std::string res; diff --git a/src/libs/core/include/core/String.hpp b/src/libs/core/include/core/String.hpp index f096c8e2..62010795 100644 --- a/src/libs/core/include/core/String.hpp +++ b/src/libs/core/include/core/String.hpp @@ -105,6 +105,9 @@ namespace lms::core::stringUtils void writeJSEscapedString(std::ostream& os, std::string_view str); void writeJsonEscapedString(std::ostream& os, std::string_view str); + [[nodiscard]] std::string xmlEscape(std::string_view str); + void writeXmlEscapedString(std::ostream& os, std::string_view str); + [[nodiscard]] std::string escapeString(std::string_view str, std::string_view charsToEscape, char escapeChar); [[nodiscard]] std::string unescapeString(std::string_view str, char escapeChar); diff --git a/src/libs/core/test/String.cpp b/src/libs/core/test/String.cpp index 12253fd6..29b1d259 100644 --- a/src/libs/core/test/String.cpp +++ b/src/libs/core/test/String.cpp @@ -221,6 +221,27 @@ namespace lms::core::stringUtils::tests EXPECT_EQ(jsonEscape(R"(Test'.mp3)"), R"(Test'.mp3)"); EXPECT_EQ(jsonEscape(R"(Test"".mp3)"), R"(Test\"\".mp3)"); EXPECT_EQ(jsonEscape(R"(\Test\.mp3)"), R"(\\Test\\.mp3)"); + EXPECT_EQ(jsonEscape("Line1\nLine2"), R"(Line1\nLine2)"); + EXPECT_EQ(jsonEscape("Line1\rLine2"), R"(Line1\rLine2)"); + EXPECT_EQ(jsonEscape("Col1\tCol2"), R"(Col1\tCol2)"); + EXPECT_EQ(jsonEscape("Hello\bWorld"), R"(Hello\bWorld)"); + EXPECT_EQ(jsonEscape("Hello\fWorld"), R"(Hello\fWorld)"); + EXPECT_EQ(jsonEscape("Hello\nWorld"), R"(Hello\nWorld)"); + } + + TEST(StringUtils, escapeXmlString) + { + EXPECT_EQ(xmlEscape(""), ""); + EXPECT_EQ(xmlEscape("Test.mp3"), "Test.mp3"); + EXPECT_EQ(xmlEscape("A & B"), "A & B"); + EXPECT_EQ(xmlEscape(""), "<tag>"); + EXPECT_EQ(xmlEscape(R"(He said "Hello")"), "He said "Hello""); + EXPECT_EQ(xmlEscape("It's fine"), "It's fine"); + EXPECT_EQ(xmlEscape(R"(O'Hara)"), "<tag attr="val & val2">O'Hara</tag>"); + EXPECT_EQ(xmlEscape(R"(\Test\.mp3)"), R"(\Test\.mp3)"); + EXPECT_EQ(xmlEscape("Café & Tea"), "Café & Tea"); + EXPECT_EQ(xmlEscape(R"(&<>'")"), "&<>'""); + EXPECT_EQ(xmlEscape("Line1\nLine2"), "Line1\nLine2"); } TEST(StringUtils, escapeString) diff --git a/src/libs/subsonic/CMakeLists.txt b/src/libs/subsonic/CMakeLists.txt index e82ccfce..89519d64 100644 --- a/src/libs/subsonic/CMakeLists.txt +++ b/src/libs/subsonic/CMakeLists.txt @@ -64,6 +64,10 @@ target_link_libraries(lmssubsonic PUBLIC Wt::Wt ) +if(BUILD_TESTING) + add_subdirectory(test) +endif() + if (BUILD_BENCHMARKS) add_subdirectory(bench) endif() \ No newline at end of file diff --git a/src/libs/subsonic/impl/SubsonicResponse.cpp b/src/libs/subsonic/impl/SubsonicResponse.cpp index 39609315..4427acb3 100644 --- a/src/libs/subsonic/impl/SubsonicResponse.cpp +++ b/src/libs/subsonic/impl/SubsonicResponse.cpp @@ -23,9 +23,8 @@ #include #include -#include - #include "core/String.hpp" +#include "core/Utils.hpp" #include "core/Version.hpp" #include "ProtocolVersion.hpp" @@ -113,133 +112,90 @@ namespace lms::api::subsonic setAttribute("version", std::to_string(protocolVersion.major) + "." + std::to_string(protocolVersion.minor) + "." + std::to_string(protocolVersion.patch)); } - Response Response::createOkResponse(ProtocolVersion protocolVersion) + void Response::XmlSerializer::serializeNode(std::ostream& os, const Node& node, std::string_view tagName) { - return createResponseCommon(protocolVersion); - } + // Opening tag + os << '<' << tagName; - Response Response::createFailedResponse(ProtocolVersion protocolVersion, const Error& error) - { - return createResponseCommon(protocolVersion, &error); - } - - Response Response::createResponseCommon(ProtocolVersion protocolVersion, const Error* error) - { - Response response; - Node& responseNode{ response._root.createChild("subsonic-response") }; - - responseNode.setAttribute("status", error ? "failed" : "ok"); - responseNode.setVersionAttribute(protocolVersion); - - if (error) + // Attributes + for (const auto& [key, value] : node._attributes) { - Node& errorNode{ responseNode.createChild("error") }; - errorNode.setAttribute("code", static_cast(error->getCode())); - errorNode.setAttribute("message", error->getMessage()); + os << ' ' << key.str() << '='; + os << '"'; + serializeValue(os, value); + os << '"'; } - // OpenSubsonic mandatory fields - // No big deal to send them even for legacy clients - responseNode.setAttribute("type", "lms"); - responseNode.setAttribute("serverVersion", core::getVersion()); - responseNode.setAttribute("openSubsonic", true); + // Hack + if (tagName == "subsonic-response") + os << " xmlns=\"http://subsonic.org/restapi\""; - return response; - } + bool hasChildren = !node._children.empty() || !node._childrenArrays.empty() || !node._childrenValues.empty(); + bool hasValue = node._value.has_value(); - void Response::addNode(Node::Key key, Node&& node) - { - return _root._children["subsonic-response"].addChild(key, std::move(node)); - } - - Response::Node& Response::createNode(Node::Key key) - { - return _root._children["subsonic-response"].createChild(key); - } - - Response::Node& Response::createArrayNode(Node::Key key) - { - return _root._children["subsonic-response"].createArrayChild(key); - } - - void Response::write(std::ostream& os, ResponseFormat format) const - { - switch (format) + if (!hasChildren && !hasValue) { - case ResponseFormat::xml: - writeXML(os); - break; - case ResponseFormat::json: - writeJSON(os); - break; + os << "/>"; // Self-closing tag + return; } + + os << '>'; // End opening tag + + // Node value (text content) + if (hasValue) + serializeValue(os, *node._value); + + // Child nodes + for (const auto& [key, childNode] : node._children) + serializeNode(os, childNode, key.str()); + + // Child arrays + for (const auto& [key, childArrayNodes] : node._childrenArrays) + for (const Node& childNode : childArrayNodes) + serializeNode(os, childNode, key.str()); + + // Array values + for (const auto& [key, childValues] : node._childrenValues) + { + for (const Node::ValueType& value : childValues) + { + os << '<' << key.str() << '>'; + serializeValue(os, value); + os << "'; + } + } + + // Closing tag + os << "'; + } + + void Response::XmlSerializer::serializeValue(std::ostream& os, const Node::ValueType& value) + { + std::visit(core::utils::overloads{ + [&](const Node::string& str) { core::stringUtils::writeXmlEscapedString(os, str); }, + [&](bool value) { os << (value ? "true" : "false"); }, + [&](float value) { os << value; }, + [&](long long value) { os << value; } }, + value); + } + + void Response::XmlSerializer::serializeEscapedString(std::ostream& os, std::string_view str) + { + core::stringUtils::writeXmlEscapedString(os, str); } void Response::writeXML(std::ostream& os) const { - std::function nodeToPropertyTree = [&](const Node& node) { - boost::property_tree::ptree res; + os << R"()" << '\n'; - auto valueToPropertyTree = [](const Node::ValueType& value) { - boost::property_tree::ptree res; - std::visit([&](const auto& rawValue) { - using RawValueType = std::decay_t; - if constexpr (std::is_same_v) - res.put_value(core::stringUtils::replaceInString(rawValue, "\n", "\\n")); - else - res.put_value(rawValue); - }, - value); + XmlSerializer serializer; - return res; - }; - - if (node._value) - { - res = valueToPropertyTree(*node._value); - } - else - { - for (const auto& [key, childNode] : node._children) - { - boost::property_tree::ptree& tree{ res.add_child(std::string{ key.str() }, nodeToPropertyTree(childNode)) }; - // Hardcoded attribute to simplify createOkResponse calls - if (key == "subsonic-response") - tree.put(".xmlns", "http://subsonic.org/restapi"); - } - - for (const auto& [key, childArrayNodes] : node._childrenArrays) - { - for (const Node& childNode : childArrayNodes) - res.add_child(std::string{ key.str() }, nodeToPropertyTree(childNode)); - } - - for (const auto& [key, childArrayValues] : node._childrenValues) - { - for (const Response::Node::ValueType& value : childArrayValues) - res.add_child(std::string{ key.str() }, valueToPropertyTree(value)); - } - } - - for (const auto& [key, value] : node._attributes) - { - if (std::holds_alternative(value)) - res.put("." + std::string{ key.str() }, std::get(value)); - else if (std::holds_alternative(value)) - res.put("." + std::string{ key.str() }, std::get(value)); - else if (std::holds_alternative(value)) - res.put("." + std::string{ key.str() }, std::get(value)); - else if (std::holds_alternative(value)) - res.put("." + std::string{ key.str() }, std::get(value)); - else - assert(false); - } - - return res; - }; - - const boost::property_tree::ptree root{ nodeToPropertyTree(_root) }; - boost::property_tree::write_xml(os, root); + assert(_root._children.size() == 1); + if (_root._children.size() == 1) + { + const auto& [tagName, node] = *_root._children.begin(); + serializer.serializeNode(os, node, tagName.str()); + } } void Response::JsonSerializer::serializeNode(std::ostream& os, const Response::Node& node) @@ -325,30 +281,18 @@ namespace lms::api::subsonic void Response::JsonSerializer::serializeValue(std::ostream& os, const Node::ValueType& value) { - if (std::holds_alternative(value)) - { - serializeEscapedString(os, std::get(value)); - } - else if (std::holds_alternative(value)) - { - os << (std::get(value) ? "true" : "false"); - } - else if (std::holds_alternative(value)) - { - const float d{ std::get(value) }; - if (std::isnan(d) || std::fabs(d) == std::numeric_limits::infinity()) - os << "null"; - else - os << d; - } - else if (std::holds_alternative(value)) - { - os << std::get(value); - } - else - { - assert(false); - } + std::visit( + core::utils::overloads{ + [&](const Node::string& str) { serializeEscapedString(os, str); }, + [&](bool value) { os << (value ? "true" : "false"); }, + [&](float value) { + if (std::isnan(value) || std::fabs(value) == std::numeric_limits::infinity()) + os << "null"; + else + os << value; + }, + [&](long long value) { os << value; } }, + value); } void Response::JsonSerializer::serializeEscapedString(std::ostream& os, std::string_view str) @@ -358,6 +302,68 @@ namespace lms::api::subsonic os << '\"'; } + Response Response::createOkResponse(ProtocolVersion protocolVersion) + { + return createResponseCommon(protocolVersion); + } + + Response Response::createFailedResponse(ProtocolVersion protocolVersion, const Error& error) + { + return createResponseCommon(protocolVersion, &error); + } + + Response Response::createResponseCommon(ProtocolVersion protocolVersion, const Error* error) + { + Response response; + Node& responseNode{ response._root.createChild("subsonic-response") }; + + responseNode.setAttribute("status", error ? "failed" : "ok"); + responseNode.setVersionAttribute(protocolVersion); + + if (error) + { + Node& errorNode{ responseNode.createChild("error") }; + errorNode.setAttribute("code", static_cast(error->getCode())); + errorNode.setAttribute("message", error->getMessage()); + } + + // OpenSubsonic mandatory fields + // No big deal to send them even for legacy clients + responseNode.setAttribute("type", "lms"); + responseNode.setAttribute("serverVersion", core::getVersion()); + responseNode.setAttribute("openSubsonic", true); + + return response; + } + + void Response::addNode(Node::Key key, Node&& node) + { + return _root._children["subsonic-response"].addChild(key, std::move(node)); + } + + Response::Node& Response::createNode(Node::Key key) + { + return _root._children["subsonic-response"].createChild(key); + } + + Response::Node& Response::createArrayNode(Node::Key key) + { + return _root._children["subsonic-response"].createArrayChild(key); + } + + void Response::write(std::ostream& os, ResponseFormat format) const + { + switch (format) + { + case ResponseFormat::xml: + writeXML(os); + break; + case ResponseFormat::json: + writeJSON(os); + break; + } + } + void Response::writeJSON(std::ostream& os) const { JsonSerializer serializer; diff --git a/src/libs/subsonic/impl/SubsonicResponse.hpp b/src/libs/subsonic/impl/SubsonicResponse.hpp index 1eec8665..44f19387 100644 --- a/src/libs/subsonic/impl/SubsonicResponse.hpp +++ b/src/libs/subsonic/impl/SubsonicResponse.hpp @@ -57,6 +57,11 @@ namespace lms::api::subsonic Error(Code code) : _code{ code } {} + virtual ~Error() = default; + + Error(const Error&) = delete; + Error& operator=(const Error&) = delete; + virtual std::string getMessage() const = 0; Code getCode() const { return _code; } @@ -318,8 +323,16 @@ namespace lms::api::subsonic { public: void serializeNode(std::ostream& os, const Node& node); - void serializeValue(std::ostream& os, const Node::ValueType& value); - void serializeEscapedString(std::ostream&, std::string_view str); + static void serializeValue(std::ostream& os, const Node::ValueType& value); + static void serializeEscapedString(std::ostream&, std::string_view str); + }; + + class XmlSerializer + { + public: + void serializeNode(std::ostream& os, const Node& node, std::string_view tagName); + static void serializeValue(std::ostream& os, const Node::ValueType& value); + static void serializeEscapedString(std::ostream&, std::string_view str); }; void writeJSON(std::ostream& os) const; diff --git a/src/libs/subsonic/test/CMakeLists.txt b/src/libs/subsonic/test/CMakeLists.txt new file mode 100644 index 00000000..6e79cf78 --- /dev/null +++ b/src/libs/subsonic/test/CMakeLists.txt @@ -0,0 +1,20 @@ +include(GoogleTest) + +add_executable(test-subsonic + SubsonicResponseTest.cpp + ) + +target_include_directories(test-subsonic PRIVATE + ../impl + ) + +target_link_libraries(test-subsonic PRIVATE + lmscore + lmssubsonic + GTest::GTest + ) + +if (NOT CMAKE_CROSSCOMPILING) + gtest_discover_tests(test-subsonic) +endif() + diff --git a/src/libs/subsonic/test/SubsonicResponseTest.cpp b/src/libs/subsonic/test/SubsonicResponseTest.cpp new file mode 100644 index 00000000..3bfa4a65 --- /dev/null +++ b/src/libs/subsonic/test/SubsonicResponseTest.cpp @@ -0,0 +1,107 @@ +/* + * Copyright (C) 2025 Emeric Poupon + * + * This file is part of LMS. + * + * LMS is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * LMS is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with LMS. If not, see . + */ + +#include + +#include + +#include "ProtocolVersion.hpp" +#include "SubsonicResponse.hpp" + +namespace lms::api::subsonic::tests +{ + namespace + { + Response generateFakeResponse() + { + Response response{ Response::createOkResponse(defaultServerProtocolVersion) }; + + Response::Node& node{ response.createNode("MyNode") }; + node.setAttribute("Attr1", "value1"); + node.setAttribute("Attr2", "value2"); + node.setAttribute("attr3", ""); + node.setAttribute("attr4", true); + node.setAttribute("attr5", false); + node.setAttribute("attr6", 3.14159265359); + node.setAttribute("attr7", 333666); + + for (std::size_t i{}; i < 2; ++i) + { + Response::Node& childNode{ node.createArrayChild("MyArrayChild") }; + childNode.setAttribute("Attr42", i); + + node.addArrayValue("MyArray1", "value1"); + node.addArrayValue("MyArray1", "value2"); + for (std::size_t j{}; j < i; ++j) + node.addArrayValue("MyArray2", j); + } + + return response; + } + } // namespace + + TEST(SubsonicResponse, emptyJson) + { + Response response{ Response::createOkResponse(ProtocolVersion{ 1, 16, 0 }) }; + + std::ostringstream oss; + response.write(oss, ResponseFormat::json); + + EXPECT_EQ(oss.str(), R"({"subsonic-response":{"openSubsonic":true,"serverVersion":"v3.70.0","status":"ok","type":"lms","version":"1.16.0"}})"); + } + + TEST(SubsonicResponse, json) + { + Response response{ generateFakeResponse() }; + + std::ostringstream oss; + response.write(oss, ResponseFormat::json); + + EXPECT_EQ(oss.str(), R"({"subsonic-response":{"openSubsonic":true,"serverVersion":"v3.70.0","status":"ok","type":"lms","version":"1.16.0","MyNode":{"Attr1":"value1","Attr2":"value2","attr3":"","attr4":true,"attr5":false,"attr6":3.14159,"attr7":333666,"MyArrayChild":[{"Attr42":0},{"Attr42":1}],"MyArray1":["value1","value2","value1","value2"],"MyArray2":[0]}}})"); + } + + TEST(SubsonicResponse, emptyXml) + { + Response response{ Response::createOkResponse(ProtocolVersion{ 1, 16, 0 }) }; + + std::ostringstream oss; + response.write(oss, ResponseFormat::xml); + + EXPECT_EQ(oss.str(), R"( +)"); + } + + TEST(SubsonicResponse, xml) + { + Response response{ generateFakeResponse() }; + + std::ostringstream oss; + response.write(oss, ResponseFormat::xml); + + EXPECT_EQ(oss.str(), R"( +value1value2value1value20)"); + } + +} // namespace lms::api::subsonic::tests + +int main(int argc, char** argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} \ No newline at end of file