lib/upnp/Action: common UpnpSendAction() wrapper for pupnp and npupnp

Merge a lot of duplicate code.

src/db/plugins/upnp/ContentDirectoryService.cxx

@@ -2,33 +2,21 @@
 // Copyright The Music Player Daemon Project
 
 #include "lib/upnp/ContentDirectoryService.hxx"
-#include "lib/upnp/Error.hxx"
+#include "lib/upnp/Action.hxx"
 #include "Directory.hxx"
 #include "util/NumberParser.hxx"
-#include "util/ScopeExit.hxx"
-#include "config.h"
-
-#ifdef USING_PUPNP
-#include "lib/upnp/ixmlwrap.hxx"
-#include "lib/upnp/Action.hxx"
-#include "lib/upnp/UniqueIxml.hxx"
-#endif
 
 #include <fmt/format.h>
 
-#include <algorithm>
-
-#ifdef USING_PUPNP
 static void
-ReadResultTag(UPnPDirContent &dirbuf, IXML_Document *response)
+ReadResultTag(UPnPDirContent &dirbuf, const UpnpActionResponse &response)
 {
-	const char *p = ixmlwrap::getFirstElementValue(response, "Result");
+	const char *p = response.GetValue("Result");
 	if (p == nullptr)
 		p = "";
 
 	dirbuf.Parse(p);
 }
-#endif
 
 inline void
 ContentDirectoryService::readDirSlice(UpnpClient_Handle hdl,
@@ -37,71 +25,28 @@ ContentDirectoryService::readDirSlice(UpnpClient_Handle hdl,
 				      unsigned &didreadp,
 				      unsigned &totalp) const
 {
-#ifdef USING_PUPNP
 	// Some devices require an empty SortCriteria, else bad params
-	IXML_Document *request =
+	const auto response = UpnpSendAction(hdl, m_actionURL.c_str(),
-		MakeActionHelper("Browse", m_serviceType.c_str(),
+					     "Browse", m_serviceType.c_str(),
-				 "ObjectID", objectId,
+					     {
-				 "BrowseFlag", "BrowseDirectChildren",
-				 "Filter", "*",
-				 "SortCriteria", "",
-				 "StartingIndex",
-				 fmt::format_int{offset}.c_str(),
-				 "RequestedCount",
-				 fmt::format_int{count}.c_str());
-	if (request == nullptr)
-		throw std::runtime_error("UpnpMakeAction() failed");
-
-	AtScopeExit(request) { ixmlDocument_free(request); };
-
-	IXML_Document *response;
-	int code = UpnpSendAction(hdl, m_actionURL.c_str(), m_serviceType.c_str(),
-				  nullptr /*devUDN*/, request, &response);
-	if (code != UPNP_E_SUCCESS)
-		throw Upnp::MakeError(code, "UpnpSendAction() failed");
-
-	AtScopeExit(response) { ixmlDocument_free(response); };
-
-	const char *value = ixmlwrap::getFirstElementValue(response, "NumberReturned");
-	didreadp = value != nullptr
-		? ParseUnsigned(value)
-		: 0;
-
-	value = ixmlwrap::getFirstElementValue(response, "TotalMatches");
-	if (value != nullptr)
-		totalp = ParseUnsigned(value);
-
-	ReadResultTag(dirbuf, response);
-#else
-	std::vector<std::pair<std::string, std::string>> actionParams{
 						     {"ObjectID", objectId},
 						     {"BrowseFlag", "BrowseDirectChildren"},
 						     {"Filter", "*"},
 						     {"SortCriteria", ""},
 						     {"StartingIndex", fmt::format_int{offset}.c_str()},
 						     {"RequestedCount", fmt::format_int{count}.c_str()},
-	};
+					     });
-	std::vector<std::pair<std::string, std::string>> responseData;
-	int errcode;
-	std::string errdesc;
-	int code = UpnpSendAction(hdl, "", m_actionURL, m_serviceType, "Browse",
-				  actionParams, responseData, &errcode, errdesc);
-	if (code != UPNP_E_SUCCESS)
-		throw Upnp::MakeError(code, "UpnpSendAction() failed");
 
-	const char *p = "";
+	const char *value = response.GetValue("NumberReturned");
-	didreadp = 0;
+	didreadp = value != nullptr
-	for (const auto &entry : responseData) {
+		? ParseUnsigned(value)
-		if (entry.first == "Result") {
+		: 0;
-			p = entry.second.c_str();
+
-		} else if (entry.first == "TotalMatches") {
+	value = response.GetValue("TotalMatches");
-			totalp = ParseUnsigned(entry.second.c_str());
+	if (value != nullptr)
-		} else if (entry.first == "NumberReturned") {
+		totalp = ParseUnsigned(value);
-			didreadp = ParseUnsigned(entry.second.c_str());
+
-		}
+	ReadResultTag(dirbuf, response);
-	}
-	dirbuf.Parse(p);
-#endif
 }
 
 UPnPDirContent
@@ -130,73 +75,29 @@ ContentDirectoryService::search(UpnpClient_Handle hdl,
 	unsigned offset = 0, total = -1, count;
 
 	do {
-#ifdef USING_PUPNP
+		const auto response = UpnpSendAction(hdl, m_actionURL.c_str(),
-		UniqueIxmlDocument request(MakeActionHelper("Search", m_serviceType.c_str(),
+						     "Search", m_serviceType.c_str(),
-							    "ContainerID", objectId,
+						     {
-							    "SearchCriteria", ss,
+							     {"ContainerID", objectId},
-							    "Filter", "*",
+							     {"SearchCriteria", ss},
-							    "SortCriteria", "",
+							     {"Filter", "*"},
-							    "StartingIndex",
+							     {"SortCriteria", ""},
-							    fmt::format_int{offset}.c_str(),
+							     {"StartingIndex", fmt::format_int{offset}.c_str()},
-							    "RequestedCount", "0")); // Setting a value here gets twonky into fits
+							     {"RequestedCount", "0"}, // Setting a value here gets twonky into fits
-		if (!request)
+						     });
-			throw std::runtime_error("UpnpMakeAction() failed");
 
-		IXML_Document *_response;
+		const char *value = response.GetValue("NumberReturned");
-		auto code = UpnpSendAction(hdl, m_actionURL.c_str(),
-					   m_serviceType.c_str(),
-					   nullptr /*devUDN*/,
-					   request.get(), &_response);
-		if (code != UPNP_E_SUCCESS)
-			throw Upnp::MakeError(code, "UpnpSendAction() failed");
-
-		UniqueIxmlDocument response(_response);
-
-		const char *value =
-			ixmlwrap::getFirstElementValue(response.get(),
-						       "NumberReturned");
 		count = value != nullptr
 			? ParseUnsigned(value)
 			: 0;
 
 		offset += count;
 
-		value = ixmlwrap::getFirstElementValue(response.get(),
+		value = response.GetValue("TotalMatches");
-						       "TotalMatches");
 		if (value != nullptr)
 			total = ParseUnsigned(value);
 
-		ReadResultTag(dirbuf, response.get());
+		ReadResultTag(dirbuf, response);
-#else
-		std::vector<std::pair<std::string, std::string>> actionParams{
-			{"ContainerID", objectId},
-			{"SearchCriteria", ss},
-			{"Filter", "*"},
-			{"SortCriteria", ""},
-			{"StartingIndex", fmt::format_int{offset}.c_str()},
-			{"RequestedCount", "0"}};
-		std::vector<std::pair<std::string, std::string>> responseData;
-		int errcode;
-		std::string errdesc;
-		int code = UpnpSendAction(hdl, "", m_actionURL, m_serviceType, "Search",
-					  actionParams, responseData, &errcode, errdesc);
-		if (code != UPNP_E_SUCCESS)
-			throw Upnp::MakeError(code, "UpnpSendAction() failed");
-
-		const char *p = "";
-		count = 0;
-		for (const auto &entry : responseData) {
-			if (entry.first == "Result") {
-				p = entry.second.c_str();
-			} else if (entry.first == "TotalMatches") {
-				total = ParseUnsigned(entry.second.c_str());
-			} else if (entry.first == "NumberReturned") {
-				count = ParseUnsigned(entry.second.c_str());
-				offset += count;
-			}
-		}
-		dirbuf.Parse(p);
-#endif
 	} while (count > 0 && offset < total);
 
 	return dirbuf;
@@ -206,46 +107,18 @@ UPnPDirContent
 ContentDirectoryService::getMetadata(UpnpClient_Handle hdl,
 				     const char *objectId) const
 {
-#ifdef USING_PUPNP
+	const auto response = UpnpSendAction(hdl, m_actionURL.c_str(),
-	// Create request
+					     "Browse", m_serviceType.c_str(),
-	UniqueIxmlDocument request(MakeActionHelper("Browse", m_serviceType.c_str(),
+					     {
-						    "ObjectID", objectId,
+						     {"ObjectID", objectId},
-						    "BrowseFlag", "BrowseMetadata",
+						     {"BrowseFlag", "BrowseMetadata"},
-						    "Filter", "*",
+						     {"Filter", "*"},
-						    "SortCriteria", "",
+						     {"SortCriteria", ""},
-						    "StartingIndex", "0",
+						     {"StartingIndex", "0"},
-						    "RequestedCount", "1"));
+						     {"RequestedCount", "1"},
-	if (request == nullptr)
+					     });
-		throw std::runtime_error("UpnpMakeAction() failed");
 
-	IXML_Document *_response;
-	auto code = UpnpSendAction(hdl, m_actionURL.c_str(),
-				   m_serviceType.c_str(),
-				   nullptr /*devUDN*/, request.get(), &_response);
-	if (code != UPNP_E_SUCCESS)
-		throw Upnp::MakeError(code, "UpnpSendAction() failed");
-
-	UniqueIxmlDocument response(_response);
 	UPnPDirContent dirbuf;
-	ReadResultTag(dirbuf, response.get());
+	ReadResultTag(dirbuf, response);
 	return dirbuf;
-#else
-	std::vector<std::pair<std::string, std::string>> actionParams{
-		{"ObjectID", objectId}, {"BrowseFlag", "BrowseMetadata"},
-		{"Filter", "*"},	{"SortCriteria", ""},
-		{"StartingIndex", "0"}, {"RequestedCount", "1"}};
-	std::vector<std::pair<std::string, std::string>> responseData;
-	int errcode;
-	std::string errdesc;
-	int code = UpnpSendAction(hdl, "", m_actionURL, m_serviceType, "Browse",
-				  actionParams, responseData, &errcode, errdesc);
-	if (code != UPNP_E_SUCCESS)
-		throw Upnp::MakeError(code, "UpnpSendAction() failed");
-
-	auto it = std::find_if(responseData.begin(), responseData.end(), [](auto&& entry){ return entry.first == "Result"; });
-	const char *p = it != responseData.end() ? it->second.c_str() : "";
-	UPnPDirContent dirbuf;
-	dirbuf.Parse(p);
-	return dirbuf;
-#endif
 }

src/lib/upnp/Action.cxx

@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright The Music Player Daemon Project
+
+#include "Action.hxx"
+#include "Error.hxx"
+
+#ifdef USING_PUPNP
+#include "util/ScopeExit.hxx"
+
+#include <upnptools.h>
+
+static IXML_Document *
+UpnpMakeAction(const char *action_name, const char *service_type,
+	       std::initializer_list<std::pair<const char *, const char *>> args)
+{
+	IXML_Document *doc = UpnpMakeAction(action_name, service_type, 0, nullptr, nullptr);
+
+	for (const auto &[name, value] : args)
+		UpnpAddToAction(&doc, action_name, service_type, name, value);
+
+	return doc;
+}
+
+const char *
+UpnpActionResponse::GetValue(const char *name) const noexcept
+{
+	IXML_NodeList *nodes = ixmlDocument_getElementsByTagName(document, name);
+	if (!nodes)
+		return nullptr;
+
+	AtScopeExit(nodes) { ixmlNodeList_free(nodes); };
+
+	IXML_Node *first = ixmlNodeList_item(nodes, 0);
+	if (!first)
+		return nullptr;
+
+	IXML_Node *dnode = ixmlNode_getFirstChild(first);
+	if (!dnode)
+		return nullptr;
+
+	return ixmlNode_getNodeValue(dnode);
+}
+
+UpnpActionResponse
+UpnpSendAction(UpnpClient_Handle handle, const char *url,
+	       const char *action_name, const char *service_type,
+	       std::initializer_list<std::pair<const char *, const char *>> args)
+{
+	IXML_Document *request = UpnpMakeAction(action_name, service_type, args);
+	AtScopeExit(request) { ixmlDocument_free(request); };
+
+	IXML_Document *response;
+	int code = UpnpSendAction(handle, url, service_type, nullptr,
+				  request, &response);
+	if (code != UPNP_E_SUCCESS)
+		throw Upnp::MakeError(code, "UpnpSendAction() failed");
+
+	return UpnpActionResponse{response};
+}
+
+#else // USING_PUPNP
+
+UpnpActionResponse
+UpnpSendAction(UpnpClient_Handle handle, const char *url,
+	       const char *action_name, const char *service_type,
+	       const std::vector<std::pair<std::string, std::string>> &args)
+{
+	std::vector<std::pair<std::string, std::string>> params{args};
+	std::vector<std::pair<std::string, std::string>> response;
+
+	int errcode;
+	std::string errdesc;
+	int code = UpnpSendAction(handle, "", url, service_type, action_name,
+				  params, response, &errcode, errdesc);
+	if (code != UPNP_E_SUCCESS)
+		throw Upnp::MakeError(code, "UpnpSendAction() failed");
+
+	return UpnpActionResponse{std::move(response)};
+}
+
+#endif // !USING_PUPNP

src/lib/upnp/Action.hxx

@@ -3,33 +3,67 @@
 
 #pragma once
 
-#include <upnptools.h>
+#include "config.h" // for USING_PUPNP
 
-static constexpr unsigned
+#include <upnp.h> // for UpnpClient_Handle
-CountNameValuePairs() noexcept
+
-{
+#include <utility> // for std::pair
-	return 0;
+
+#ifdef USING_PUPNP
+
+#include <initializer_list>
+
+class UpnpActionResponse {
+	IXML_Document *document;
+
+public:
+	explicit UpnpActionResponse(IXML_Document *_document) noexcept
+		:document(_document) {}
+
+	~UpnpActionResponse() noexcept {
+		ixmlDocument_free(document);
 	}
 
-template<typename... Args>
+	UpnpActionResponse(const UpnpActionResponse &) = delete;
-static constexpr unsigned
+	UpnpActionResponse &operator=(const UpnpActionResponse &) = delete;
-CountNameValuePairs([[maybe_unused]] const char *name, [[maybe_unused]] const char *value,
-		    Args... args) noexcept
-{
-	return 1 + CountNameValuePairs(args...);
-}
 
-/**
+	[[gnu::pure]]
- * A wrapper for UpnpMakeAction() that counts the number of name/value
+	const char *GetValue(const char *name) const noexcept;
- * pairs and adds the nullptr sentinel.
+};
- */
+
-template<typename... Args>
+UpnpActionResponse
-static inline IXML_Document *
+UpnpSendAction(UpnpClient_Handle handle, const char *url,
-MakeActionHelper(const char *action_name, const char *service_type,
+	       const char *action_name, const char *service_type,
-		 Args... args) noexcept
+	       std::initializer_list<std::pair<const char *, const char *>> args);
-{
+
-	const unsigned n = CountNameValuePairs(args...);
+#else // USING_PUPNP
-	return UpnpMakeAction(action_name, service_type, n,
+
-			      args...,
+#include <string>
-			      nullptr, nullptr);
+#include <vector>
+
+class UpnpActionResponse {
+	std::vector<std::pair<std::string, std::string>> data;
+
+public:
+	explicit UpnpActionResponse(std::vector<std::pair<std::string, std::string>> &&_data) noexcept
+		:data(_data) {}
+
+	UpnpActionResponse(const UpnpActionResponse &) = delete;
+	UpnpActionResponse &operator=(const UpnpActionResponse &) = delete;
+
+	[[gnu::pure]]
+	const char *GetValue(std::string_view name) const noexcept {
+		for (const auto &i : data)
+			if (i.first == name)
+				return i.second.c_str();
+
+		return nullptr;
 	}
+};
+
+UpnpActionResponse
+UpnpSendAction(UpnpClient_Handle handle, const char *url,
+	       const char *action_name, const char *service_type,
+	       const std::vector<std::pair<std::string, std::string>> &args);
+
+#endif // !USING_PUPNP

src/lib/upnp/ContentDirectoryService.cxx

@@ -2,23 +2,13 @@
 // Copyright The Music Player Daemon Project
 
 #include "ContentDirectoryService.hxx"
+#include "Action.hxx"
 #include "Device.hxx"
-#include "Error.hxx"
 #include "util/IterableSplitString.hxx"
 #include "util/UriRelative.hxx"
 #include "util/UriUtil.hxx"
 #include "config.h"
 
-#ifdef USING_PUPNP
-#include "ixmlwrap.hxx"
-#include "Action.hxx"
-#include "UniqueIxml.hxx"
-#endif
-
-#include <algorithm>
-
-#include <upnptools.h>
-
 using std::string_view_literals::operator""sv;
 
 ContentDirectoryService::ContentDirectoryService(const UPnPDevice &device,
@@ -42,41 +32,11 @@ ContentDirectoryService::~ContentDirectoryService() noexcept = default;
 std::forward_list<std::string>
 ContentDirectoryService::getSearchCapabilities(UpnpClient_Handle hdl) const
 {
-#ifdef USING_PUPNP
+	const auto response = UpnpSendAction(hdl, m_actionURL.c_str(),
-	UniqueIxmlDocument request(UpnpMakeAction("GetSearchCapabilities", m_serviceType.c_str(),
+					     "GetSearchCapabilities", m_serviceType.c_str(),
-						  0,
+					     {});
-						  nullptr, nullptr));
-	if (!request)
-		throw std::runtime_error("UpnpMakeAction() failed");
 
-	IXML_Document *_response;
+	const char *s = response.GetValue("SearchCaps");
-	auto code = UpnpSendAction(hdl, m_actionURL.c_str(),
-				   m_serviceType.c_str(),
-				   nullptr /*devUDN*/, request.get(), &_response);
-	if (code != UPNP_E_SUCCESS)
-		throw Upnp::MakeError(code, "UpnpSendAction() failed");
-
-	UniqueIxmlDocument response(_response);
-
-	const char *s = ixmlwrap::getFirstElementValue(response.get(),
-						       "SearchCaps");
-#else
-	std::vector<std::pair<std::string, std::string>> responseData;
-	int errcode;
-	std::string errdesc;
-	auto code = UpnpSendAction(hdl, "", m_actionURL, m_serviceType,
-				   "GetSearchCapabilities", {}, responseData, &errcode,
-				   errdesc);
-	if (code != UPNP_E_SUCCESS)
-		throw Upnp::MakeError(code, "UpnpSendAction() failed");
-
-	const char *s{nullptr};
-	for (auto &entry : responseData) {
-		if (entry.first == "SearchCaps") {
-			s = entry.second.c_str();
-		}
-	}
-#endif
 	if (s == nullptr || *s == 0)
 		return {};
 

src/lib/upnp/UniqueIxml.hxx

@@ -1,21 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-// Copyright The Music Player Daemon Project
-
-#pragma once
-
-#include <ixml.h>
-
-#include <memory>
-
-struct UpnpIxmlDeleter {
-	void operator()(IXML_Document *doc) noexcept {
-		ixmlDocument_free(doc);
-	}
-
-	void operator()(IXML_NodeList *nl) noexcept {
-		ixmlNodeList_free(nl);
-	}
-};
-
-typedef std::unique_ptr<IXML_Document, UpnpIxmlDeleter> UniqueIxmlDocument;
-typedef std::unique_ptr<IXML_NodeList, UpnpIxmlDeleter> UniqueIxmlNodeList;

src/lib/upnp/ixmlwrap.cxx

@@ -1,31 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-// Copyright J.F.Dockes
-
-#include "config.h"
-
-#ifdef USING_PUPNP
-#	include "ixmlwrap.hxx"
-#	include "UniqueIxml.hxx"
-
-namespace ixmlwrap {
-
-const char *
-getFirstElementValue(IXML_Document *doc, const char *name) noexcept
-{
-	UniqueIxmlNodeList nodes(ixmlDocument_getElementsByTagName(doc, name));
-	if (!nodes)
-		return nullptr;
-
-	IXML_Node *first = ixmlNodeList_item(nodes.get(), 0);
-	if (!first)
-		return nullptr;
-
-	IXML_Node *dnode = ixmlNode_getFirstChild(first);
-	if (!dnode)
-		return nullptr;
-
-	return ixmlNode_getNodeValue(dnode);
-}
-
-} // namespace ixmlwrap
-#endif

src/lib/upnp/ixmlwrap.hxx

@@ -1,17 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-// Copyright J.F.Dockes
-
-#pragma once
-
-#include <ixml.h>
-
-namespace ixmlwrap {
-	/**
-	 * Retrieve the text content for the first element of given
-	 * name.  Returns nullptr if the element does not
-	 * contain a text node
-	 */
-	const char *getFirstElementValue(IXML_Document *doc,
-					 const char *name) noexcept;
-
-}

src/lib/upnp/meson.build

@@ -47,8 +47,8 @@ upnp = static_library(
   'Device.cxx',
   'ContentDirectoryService.cxx',
   'Discovery.cxx',
-  'ixmlwrap.cxx',
   'Util.cxx',
+  'Action.cxx',
   include_directories: inc,
   dependencies: [
     log_dep,