db/Print: move sort/window emulation code to class DatabaseVisitorHelper

That way, each plugin can decide to implement it better.

Makefile.am

@@ -947,6 +947,7 @@ libdb_plugins_a_SOURCES = \
 	src/PlaylistDatabase.cxx src/PlaylistDatabase.hxx \
 	src/db/Registry.cxx src/db/Registry.hxx \
 	src/db/Helpers.cxx src/db/Helpers.hxx \
+	src/db/VHelper.cxx src/db/VHelper.hxx \
 	src/db/UniqueTags.cxx src/db/UniqueTags.hxx \
 	src/db/plugins/simple/DatabaseSave.cxx \
 	src/db/plugins/simple/DatabaseSave.hxx \

src/db/DatabasePrint.cxx

@@ -144,38 +144,6 @@ PrintPlaylistFull(Response &r, bool base,
 		time_print(r, "Last-Modified", playlist.mtime);
 }
 
-gcc_pure
-static bool
-CompareNumeric(const char *a, const char *b) noexcept
-{
-	long a_value = strtol(a, nullptr, 10);
-	long b_value = strtol(b, nullptr, 10);
-
-	return a_value < b_value;
-}
-
-gcc_pure
-static bool
-CompareTags(TagType type, bool descending, const Tag &a, const Tag &b) noexcept
-{
-	const char *a_value = a.GetSortValue(type);
-	const char *b_value = b.GetSortValue(type);
-
-	if (descending) {
-		using std::swap;
-		swap(a_value, b_value);
-	}
-
-	switch (type) {
-	case TAG_DISC:
-	case TAG_TRACK:
-		return CompareNumeric(a_value, b_value);
-
-	default:
-		return strcmp(a_value, b_value) < 0;
-	}
-}
-
 void
 db_selection_print(Response &r, Partition &partition,
 		   const DatabaseSelection &selection,
@@ -195,66 +163,7 @@ db_selection_print(Response &r, Partition &partition,
 			    std::ref(r), base, _1, _2)
 		: VisitPlaylist();
 
-	const auto window = selection.window;
+	db.Visit(selection, d, s, p);
-
-	if (selection.sort == TAG_NUM_OF_ITEM_TYPES) {
-		unsigned i = 0;
-		if (!selection.window.IsAll())
-			s = [s, window, &i](const LightSong &song){
-				if (window.Contains(i++))
-					s(song);
-			};
-
-		db.Visit(selection, d, s, p);
-	} else {
-		// TODO: allow the database plugin to sort internally
-
-		/* the client has asked us to sort the result; this is
-		   pretty expensive, because instead of streaming the
-		   result to the client, we need to copy it all into
-		   this std::vector, and then sort it */
-		std::vector<DetachedSong> songs;
-
-		{
-			auto collect_songs = [&songs](const LightSong &song){
-				songs.emplace_back(song);
-			};
-
-			db.Visit(selection, d, collect_songs, p);
-		}
-
-		const auto sort = selection.sort;
-		const auto descending = selection.descending;
-
-		if (sort == TagType(SORT_TAG_LAST_MODIFIED))
-			std::stable_sort(songs.begin(), songs.end(),
-					 [descending](const DetachedSong &a, const DetachedSong &b){
-						 return descending
-							 ? a.GetLastModified() > b.GetLastModified()
-							 : a.GetLastModified() < b.GetLastModified();
-					 });
-		else
-			std::stable_sort(songs.begin(), songs.end(),
-					 [sort, descending](const DetachedSong &a,
-							    const DetachedSong &b){
-						 return CompareTags(sort, descending,
-								    a.GetTag(),
-								    b.GetTag());
-					 });
-
-		if (window.end < songs.size())
-			songs.erase(std::next(songs.begin(), window.end),
-				    songs.end());
-
-		if (window.start >= songs.size())
-			return;
-
-		songs.erase(songs.begin(),
-			    std::next(songs.begin(), window.start));
-
-		for (const auto &song : songs)
-			s((LightSong)song);
-	}
 }
 
 static void

src/db/VHelper.cxx

@@ -0,0 +1,133 @@
+/*
+ * Copyright 2003-2018 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 "config.h"
+#include "VHelper.hxx"
+#include "song/DetachedSong.hxx"
+#include "song/LightSong.hxx"
+#include "song/Filter.hxx"
+
+#include <assert.h>
+#include <string.h>
+
+DatabaseVisitorHelper::DatabaseVisitorHelper(const DatabaseSelection &_selection,
+					     VisitSong &visit_song) noexcept
+	:selection(_selection)
+{
+	// TODO: apply URI and SongFilter
+	assert(selection.uri.empty());
+	assert(selection.filter == nullptr);
+
+	if (selection.sort != TAG_NUM_OF_ITEM_TYPES) {
+		/* the client has asked us to sort the result; this is
+		   pretty expensive, because instead of streaming the
+		   result to the client, we need to copy it all into
+		   this std::vector, and then sort it */
+
+		original_visit_song = std::move(visit_song);
+		visit_song = [this](const auto &song){
+			songs.emplace_back(song);
+		};
+	} else if (selection.window != RangeArg::All()) {
+		original_visit_song = std::move(visit_song);
+		visit_song = [this](const auto &song){
+			if (selection.window.Contains(counter++))
+				original_visit_song(song);
+		};
+	}
+}
+
+DatabaseVisitorHelper::~DatabaseVisitorHelper() noexcept = default;
+
+gcc_pure
+static bool
+CompareNumeric(const char *a, const char *b) noexcept
+{
+	long a_value = strtol(a, nullptr, 10);
+	long b_value = strtol(b, nullptr, 10);
+
+	return a_value < b_value;
+}
+
+gcc_pure
+static bool
+CompareTags(TagType type, bool descending, const Tag &a, const Tag &b) noexcept
+{
+	const char *a_value = a.GetSortValue(type);
+	const char *b_value = b.GetSortValue(type);
+
+	if (descending) {
+		using std::swap;
+		swap(a_value, b_value);
+	}
+
+	switch (type) {
+	case TAG_DISC:
+	case TAG_TRACK:
+		return CompareNumeric(a_value, b_value);
+
+	default:
+		return strcmp(a_value, b_value) < 0;
+	}
+}
+
+void
+DatabaseVisitorHelper::Commit()
+{
+	/* only needed if sorting is enabled */
+	if (selection.sort == TAG_NUM_OF_ITEM_TYPES)
+		return;
+
+	assert(original_visit_song);
+
+	/* sort the song collection */
+	const auto sort = selection.sort;
+	const auto descending = selection.descending;
+
+	if (sort == TagType(SORT_TAG_LAST_MODIFIED))
+		std::stable_sort(songs.begin(), songs.end(),
+				 [descending](const DetachedSong &a, const DetachedSong &b){
+					 return descending
+						 ? a.GetLastModified() > b.GetLastModified()
+						 : a.GetLastModified() < b.GetLastModified();
+				 });
+	else
+		std::stable_sort(songs.begin(), songs.end(),
+				 [sort, descending](const DetachedSong &a,
+						    const DetachedSong &b){
+					 return CompareTags(sort, descending,
+							    a.GetTag(),
+							    b.GetTag());
+				 });
+
+	/* apply the "window" */
+	if (selection.window.end < songs.size())
+		songs.erase(std::next(songs.begin(), selection.window.end),
+			    songs.end());
+
+	if (selection.window.start >= songs.size())
+		return;
+
+	songs.erase(songs.begin(),
+		    std::next(songs.begin(), selection.window.start));
+
+	/* now pass all songs to the original visitor callback */
+	for (const auto &song : songs)
+		original_visit_song((LightSong)song);
+}

src/db/VHelper.hxx

@@ -0,0 +1,70 @@
+/*
+ * Copyright 2003-2018 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_DATABASE_VISITOR_HELPER_HXX
+#define MPD_DATABASE_VISITOR_HELPER_HXX
+
+#include "Visitor.hxx"
+#include "Selection.hxx"
+
+#include <vector>
+
+class DetachedSong;
+
+/**
+ * This class helps implementing Database::Visit() by emulating
+ * #DatabaseSelection features that the #Database implementation
+ * doesn't have, e.g. filtering, sorting and window.
+ *
+ * To use this class, construct it, passing unsupported features and
+ * the visitor callback to the constructor; before leaving Visit(),
+ * call Commit() (unless an error has occurred).
+ */
+class DatabaseVisitorHelper {
+	const DatabaseSelection selection;
+
+	/**
+	 * If the plugin can't sort, then this container will collect
+	 * all songs, sort them and report them to the visitor in
+	 * Commit().
+	 */
+	std::vector<DetachedSong> songs;
+
+	VisitSong original_visit_song;
+
+	/**
+	 * Used to emulate the "window".
+	 */
+	unsigned counter = 0;
+
+public:
+	/**
+	 * @param selection a #DatabaseSelection instance with only
+	 * features enabled which shall be emulated by this class
+	 * @param visit_song the callback function passed to
+	 * Database::Visit(); may be replaced by this class
+	 */
+	DatabaseVisitorHelper(const DatabaseSelection &selection,
+			      VisitSong &visit_song) noexcept;
+	~DatabaseVisitorHelper() noexcept;
+
+	void Commit();
+};
+
+#endif

src/db/plugins/ProxyDatabasePlugin.cxx

@@ -23,6 +23,7 @@
 #include "db/DatabasePlugin.hxx"
 #include "db/DatabaseListener.hxx"
 #include "db/Selection.hxx"
+#include "db/VHelper.hxx"
 #include "db/DatabaseError.hxx"
 #include "db/PlaylistInfo.hxx"
 #include "db/LightDirectory.hxx"
@@ -804,6 +805,15 @@ try {
 	throw;
 }
 
+gcc_const
+static DatabaseSelection
+CheckSelection(DatabaseSelection selection) noexcept
+{
+	selection.uri.clear();
+	selection.filter = nullptr;
+	return selection;
+}
+
 void
 ProxyDatabase::Visit(const DatabaseSelection &selection,
 		     VisitDirectory visit_directory,
@@ -813,11 +823,14 @@ ProxyDatabase::Visit(const DatabaseSelection &selection,
 	// TODO: eliminate the const_cast
 	const_cast<ProxyDatabase *>(this)->EnsureConnected();
 
+	DatabaseVisitorHelper helper(CheckSelection(selection), visit_song);
+
 	if (!visit_directory && !visit_playlist && selection.recursive &&
 	    !selection.IsEmpty()) {
 		/* this optimized code path can only be used under
 		   certain conditions */
 		::SearchSongs(connection, selection, visit_song);
+		helper.Commit();
 		return;
 	}
 
@@ -825,6 +838,8 @@ ProxyDatabase::Visit(const DatabaseSelection &selection,
 	::Visit(connection, selection.uri.c_str(),
 		selection.recursive, selection.filter,
 		visit_directory, visit_song, visit_playlist);
+
+	helper.Commit();
 }
 
 void

src/db/plugins/simple/SimpleDatabasePlugin.cxx

@@ -26,6 +26,7 @@
 #include "db/Helpers.hxx"
 #include "db/Stats.hxx"
 #include "db/UniqueTags.hxx"
+#include "db/VHelper.hxx"
 #include "db/LightDirectory.hxx"
 #include "Directory.hxx"
 #include "Song.hxx"
@@ -265,6 +266,15 @@ SimpleDatabase::ReturnSong(gcc_unused const LightSong *song) const noexcept
 	}
 }
 
+gcc_const
+static DatabaseSelection
+CheckSelection(DatabaseSelection selection) noexcept
+{
+	selection.uri.clear();
+	selection.filter = nullptr;
+	return selection;
+}
+
 void
 SimpleDatabase::Visit(const DatabaseSelection &selection,
 		      VisitDirectory visit_directory,
@@ -286,6 +296,8 @@ SimpleDatabase::Visit(const DatabaseSelection &selection,
 		return;
 	}
 
+	DatabaseVisitorHelper helper(CheckSelection(selection), visit_song);
+
 	if (r.uri == nullptr) {
 		/* it's a directory */
 
@@ -295,6 +307,7 @@ SimpleDatabase::Visit(const DatabaseSelection &selection,
 		r.directory->Walk(selection.recursive, selection.filter,
 				  visit_directory, visit_song,
 				  visit_playlist);
+		helper.Commit();
 		return;
 	}
 
@@ -306,6 +319,7 @@ SimpleDatabase::Visit(const DatabaseSelection &selection,
 				if (selection.Match(song2))
 					visit_song(song2);
 
+				helper.Commit();
 				return;
 			}
 		}

src/db/plugins/upnp/UpnpDatabasePlugin.cxx

@@ -27,6 +27,7 @@
 #include "db/Interface.hxx"
 #include "db/DatabasePlugin.hxx"
 #include "db/Selection.hxx"
+#include "db/VHelper.hxx"
 #include "db/DatabaseError.hxx"
 #include "db/LightDirectory.hxx"
 #include "song/LightSong.hxx"
@@ -576,6 +577,15 @@ UpnpDatabase::VisitServer(const ContentDirectoryService &server,
 	}
 }
 
+gcc_const
+static DatabaseSelection
+CheckSelection(DatabaseSelection selection) noexcept
+{
+	selection.uri.clear();
+	selection.filter = nullptr;
+	return selection;
+}
+
 // Deal with the possibly multiple servers, call VisitServer if needed.
 void
 UpnpDatabase::Visit(const DatabaseSelection &selection,
@@ -583,6 +593,8 @@ UpnpDatabase::Visit(const DatabaseSelection &selection,
 		    VisitSong visit_song,
 		    VisitPlaylist visit_playlist) const
 {
+	DatabaseVisitorHelper helper(CheckSelection(selection), visit_song);
+
 	auto vpath = SplitString(selection.uri.c_str(), '/');
 	if (vpath.empty()) {
 		for (const auto &server : discovery->GetDirectories()) {
@@ -598,6 +610,7 @@ UpnpDatabase::Visit(const DatabaseSelection &selection,
 					    visit_playlist);
 		}
 
+		helper.Commit();
 		return;
 	}
 
@@ -608,6 +621,7 @@ UpnpDatabase::Visit(const DatabaseSelection &selection,
 	auto server = discovery->GetServer(servername.c_str());
 	VisitServer(server, std::move(vpath), selection,
 		    visit_directory, visit_song, visit_playlist);
+	helper.Commit();
 }
 
 void