DatabasePlugin: add method VisitUniqueTags()

Optimize the ProxyDatabase by invoking "list" on the peer, instead of
visiting all songs.

9 files changed, 216 insertions, 53 deletions

diff --git a/Makefile.am b/Makefile.am
index a84a2e28..51dd7fd8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -438,6 +438,7 @@ endif
 
 libdb_plugins_a_SOURCES = \
 	src/DatabaseRegistry.cxx src/DatabaseRegistry.hxx \
+	src/DatabaseHelpers.cxx src/DatabaseHelpers.hxx \
 	src/db/SimpleDatabasePlugin.cxx src/db/SimpleDatabasePlugin.hxx
 
 if HAVE_LIBMPDCLIENT
diff --git a/src/DatabaseHelpers.cxx b/src/DatabaseHelpers.cxx
new file mode 100644
index 00000000..9a093113
--- /dev/null
+++ b/src/DatabaseHelpers.cxx
@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) 2003-2012 The Music Player Daemon Project
+ * http://www.musicpd.org
+ *
+ * This program 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 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program 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 this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include "DatabaseHelpers.hxx"
+#include "DatabasePlugin.hxx"
+#include "song.h"
+#include "tag.h"
+
+#include <functional>
+#include <set>
+
+#include <string.h>
+
+struct StringLess {
+	gcc_pure
+	bool operator()(const char *a, const char *b) const {
+		return strcmp(a, b) < 0;
+	}
+};
+
+typedef std::set<const char *, StringLess> StringSet;
+
+static bool
+CollectTags(StringSet &set, enum tag_type tag_type, song &song)
+{
+	struct tag *tag = song.tag;
+	if (tag == nullptr)
+		return true;
+
+	bool found = false;
+	for (unsigned i = 0; i < tag->num_items; ++i) {
+		if (tag->items[i]->type == tag_type) {
+			set.insert(tag->items[i]->value);
+			found = true;
+		}
+	}
+
+	if (!found)
+		set.insert("");
+
+	return true;
+}
+
+bool
+VisitUniqueTags(const Database &db, const DatabaseSelection &selection,
+		enum tag_type tag_type,
+		VisitString visit_string,
+		GError **error_r)
+{
+	StringSet set;
+
+	using namespace std::placeholders;
+	const auto f = std::bind(CollectTags, std::ref(set), tag_type, _1);
+	if (!db.Visit(selection, f, error_r))
+		return false;
+
+	for (auto value : set)
+		if (!visit_string(value, error_r))
+			return false;
+
+	return true;
+}
diff --git a/src/DatabaseHelpers.hxx b/src/DatabaseHelpers.hxx
new file mode 100644
index 00000000..83db182e
--- /dev/null
+++ b/src/DatabaseHelpers.hxx
@@ -0,0 +1,36 @@
+/*
+ * Copyright (C) 2003-2012 The Music Player Daemon Project
+ * http://www.musicpd.org
+ *
+ * This program 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 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program 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 this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef MPD_MEMORY_DATABASE_PLUGIN_HXX
+#define MPD_MEMORY_DATABASE_PLUGIN_HXX
+
+#include "DatabaseVisitor.hxx"
+#include "tag.h"
+#include "gcc.h"
+
+class Database;
+struct DatabaseSelection;
+
+bool
+VisitUniqueTags(const Database &db, const DatabaseSelection &selection,
+		enum tag_type tag_type,
+		VisitString visit_string,
+		GError **error_r);
+
+#endif
diff --git a/src/DatabasePlugin.hxx b/src/DatabasePlugin.hxx
index 31b2a102..a6168b62 100644
--- a/src/DatabasePlugin.hxx
+++ b/src/DatabasePlugin.hxx
@@ -29,6 +29,10 @@
 #include "DatabaseVisitor.hxx"
 #include "gcc.h"
 
+extern "C" {
+#include "tag.h"
+}
+
 struct config_param;
 struct DatabaseSelection;
 struct db_visitor;
@@ -82,6 +86,14 @@ public:
 		   GError **error_r) const {
 		return Visit(selection, VisitDirectory(), visit_song, error_r);
 	}
+
+	/**
+	 * Visit all unique tag values.
+	 */
+	virtual bool VisitUniqueTags(const DatabaseSelection &selection,
+				     enum tag_type tag_type,
+				     VisitString visit_string,
+				     GError **error_r) const = 0;
 };
 
 struct DatabasePlugin {
diff --git a/src/DatabasePrint.cxx b/src/DatabasePrint.cxx
index d22d4db8..b58619a9 100644
--- a/src/DatabasePrint.cxx
+++ b/src/DatabasePrint.cxx
@@ -38,7 +38,6 @@ extern "C" {
 #include "DatabasePlugin.hxx"
 
 #include <functional>
-#include <set>
 
 static bool
 PrintDirectory(struct client *client, const directory &directory)
@@ -186,48 +185,19 @@ printInfoForAllIn(struct client *client, const char *uri_utf8,
 	return db_selection_print(client, selection, true, error_r);
 }
 
-struct StringLess {
-	gcc_pure
-	bool operator()(const char *a, const char *b) const {
-		return strcmp(a, b) < 0;
-	}
-};
-
-typedef std::set<const char *, StringLess> StringSet;
-
-static void
-visitTag(struct client *client, StringSet &set,
-	 song &song, enum tag_type tagType)
+static bool
+PrintSongURIVisitor(struct client *client, song &song)
 {
-	struct tag *tag = song.tag;
-	bool found = false;
-
-	if (tagType == LOCATE_TAG_FILE_TYPE) {
-		song_print_uri(client, &song);
-		return;
-	}
-
-	if (!tag)
-		return;
-
-	for (unsigned i = 0; i < tag->num_items; i++) {
-		if (tag->items[i]->type == tagType) {
-			set.insert(tag->items[i]->value);
-			found = true;
-		}
-	}
+	song_print_uri(client, &song);
 
-	if (!found)
-		set.insert("");
+	return true;
 }
 
 static bool
-unique_tags_visitor_song(struct client *client,
-			 enum tag_type tag_type,
-			 StringSet &set, song &song)
+PrintUniqueTag(struct client *client, enum tag_type tag_type,
+	       const char *value)
 {
-	visitTag(client, set, song, tag_type);
-
+	client_printf(client, "%s: %s\n", tag_item_names[tag_type], value);
 	return true;
 }
 
@@ -238,20 +208,16 @@ listAllUniqueTags(struct client *client, int type,
 {
 	const DatabaseSelection selection("", true, criteria);
 
-	StringSet set;
-
-	using namespace std::placeholders;
-	const auto f = std::bind(unique_tags_visitor_song, client,
-				 (enum tag_type)type, std::ref(set),
-				 _1);
-	if (!GetDatabase()->Visit(selection, f, error_r))
-		return false;
-
-	if (type >= 0 && type <= TAG_NUM_OF_ITEM_TYPES)
-		for (auto value : set)
-			client_printf(client, "%s: %s\n",
-				      tag_item_names[type],
-				      value);
-
-	return true;
+	if (type == LOCATE_TAG_FILE_TYPE) {
+		using namespace std::placeholders;
+		const auto f = std::bind(PrintSongURIVisitor, client, _1);
+		return GetDatabase()->Visit(selection, f, error_r);
+	} else {
+		using namespace std::placeholders;
+		const auto f = std::bind(PrintUniqueTag, client,
+					 (enum tag_type)type, _1);
+		return GetDatabase()->VisitUniqueTags(selection,
+						      (enum tag_type)type,
+						      f, error_r);
+	}
 }
diff --git a/src/DatabaseVisitor.hxx b/src/DatabaseVisitor.hxx
index 37b298a3..10f907ce 100644
--- a/src/DatabaseVisitor.hxx
+++ b/src/DatabaseVisitor.hxx
@@ -33,4 +33,6 @@ typedef std::function<bool(struct song &, GError **)> VisitSong;
 typedef std::function<bool(const playlist_metadata &, const directory &,
 			   GError **)> VisitPlaylist;
 
+typedef std::function<bool(const char *, GError **)> VisitString;
+
 #endif
diff --git a/src/db/ProxyDatabasePlugin.cxx b/src/db/ProxyDatabasePlugin.cxx
index 68ba5a54..f06728f8 100644
--- a/src/db/ProxyDatabasePlugin.cxx
+++ b/src/db/ProxyDatabasePlugin.cxx
@@ -62,6 +62,11 @@ public:
 			   VisitPlaylist visit_playlist,
 			   GError **error_r) const override;
 
+	virtual bool VisitUniqueTags(const DatabaseSelection &selection,
+				     enum tag_type tag_type,
+				     VisitString visit_string,
+				     GError **error_r) const override;
+
 protected:
 	bool Configure(const struct config_param *param, GError **error_r);
 };
@@ -97,6 +102,17 @@ static constexpr struct {
 	{ TAG_NUM_OF_ITEM_TYPES, MPD_TAG_COUNT }
 };
 
+G_GNUC_CONST
+static enum mpd_tag_type
+Convert(enum tag_type tag_type)
+{
+	for (auto i = tag_table; i->d != TAG_NUM_OF_ITEM_TYPES; ++i)
+		if (i->d == tag_type)
+			return i->s;
+
+	return MPD_TAG_COUNT;
+}
+
 static bool
 CheckError(const struct mpd_connection *connection, GError **error_r)
 {
@@ -368,6 +384,42 @@ ProxyDatabase::Visit(const DatabaseSelection &selection,
 	return success;
 }
 
+bool
+ProxyDatabase::VisitUniqueTags(const DatabaseSelection &selection,
+			       enum tag_type tag_type,
+			       VisitString visit_string,
+			       GError **error_r) const
+{
+	enum mpd_tag_type tag_type2 = Convert(tag_type);
+	if (tag_type2 == MPD_TAG_COUNT) {
+		g_set_error_literal(error_r, libmpdclient_quark(), 0,
+				    "Unsupported tag");
+		return false;
+	}
+
+	if (!mpd_search_db_tags(connection, tag_type2))
+		return CheckError(connection, error_r);
+
+	// TODO: match
+	(void)selection;
+
+	if (!mpd_search_commit(connection))
+		return CheckError(connection, error_r);
+
+	bool result = true;
+
+	struct mpd_pair *pair;
+	while (result &&
+	       (pair = mpd_recv_pair_tag(connection, tag_type2)) != nullptr) {
+		result = visit_string(pair->value, error_r);
+		mpd_return_pair(connection, pair);
+	}
+
+	return mpd_response_finish(connection) &&
+		CheckError(connection, error_r) &&
+		result;
+}
+
 const DatabasePlugin proxy_db_plugin = {
 	"proxy",
 	ProxyDatabase::Create,
diff --git a/src/db/SimpleDatabasePlugin.cxx b/src/db/SimpleDatabasePlugin.cxx
index a4077eb2..ed166de4 100644
--- a/src/db/SimpleDatabasePlugin.cxx
+++ b/src/db/SimpleDatabasePlugin.cxx
@@ -20,6 +20,7 @@
 #include "config.h"
 #include "SimpleDatabasePlugin.hxx"
 #include "DatabaseSelection.hxx"
+#include "DatabaseHelpers.hxx"
 
 extern "C" {
 #include "db_error.h"
@@ -270,6 +271,16 @@ SimpleDatabase::Visit(const DatabaseSelection &selection,
 }
 
 bool
+SimpleDatabase::VisitUniqueTags(const DatabaseSelection &selection,
+				enum tag_type tag_type,
+				VisitString visit_string,
+				GError **error_r) const
+{
+	return ::VisitUniqueTags(*this, selection, tag_type, visit_string,
+				 error_r);
+}
+
+bool
 SimpleDatabase::Save(GError **error_r)
 {
 	db_lock();
diff --git a/src/db/SimpleDatabasePlugin.hxx b/src/db/SimpleDatabasePlugin.hxx
index 1e990de1..0b7e838b 100644
--- a/src/db/SimpleDatabasePlugin.hxx
+++ b/src/db/SimpleDatabasePlugin.hxx
@@ -66,6 +66,11 @@ public:
 			   VisitPlaylist visit_playlist,
 			   GError **error_r) const override;
 
+	virtual bool VisitUniqueTags(const DatabaseSelection &selection,
+				     enum tag_type tag_type,
+				     VisitString visit_string,
+				     GError **error_r) const override;
+
 protected:
 	bool Configure(const struct config_param *param, GError **error_r);