SongFilter: make Item an interface

Prepare to allow more complex expressions.

src/SongFilter.cxx

@@ -1,5 +1,5 @@
 /*
- * Copyright 2003-2017 The Music Player Daemon Project
+ * Copyright 2003-2018 The Music Player Daemon Project
  * http://www.musicpd.org
  *
  * This program is free software; you can redistribute it and/or modify
@@ -78,74 +78,76 @@ StringFilter::Match(const char *s) const noexcept
 	}
 }
 
-SongFilter::Item::Item(unsigned _tag, std::string &&_value, bool _fold_case)
+std::string
-	:tag(_tag), string_filter(std::move(_value), _fold_case)
+UriSongFilter::ToExpression() const noexcept
 {
+	return std::string("(" LOCATE_TAG_FILE_KEY " ") + (negated ? "!=" : "==") + " \"" + filter.GetValue() + "\")";
 }
 
-SongFilter::Item::Item(unsigned _tag,
+bool
-		       std::chrono::system_clock::time_point _time)
+UriSongFilter::Match(const LightSong &song) const noexcept
-	:tag(_tag), time(_time)
 {
+	return filter.Match(song.GetURI().c_str()) != negated;
 }
 
 std::string
-SongFilter::Item::ToExpression() const noexcept
+BaseSongFilter::ToExpression() const noexcept
 {
-	switch (tag) {
+	return "(base \"" + value + "\")";
-	case LOCATE_TAG_FILE_TYPE:
-		return std::string("(" LOCATE_TAG_FILE_KEY " ") + (IsNegated() ? "!=" : "==") + " \"" + string_filter.GetValue() + "\")";
-
-	case LOCATE_TAG_BASE_TYPE:
-		return "(base \"" + string_filter.GetValue() + "\")";
-
-	case LOCATE_TAG_MODIFIED_SINCE:
-		return "(modified-since \"" + string_filter.GetValue() + "\")";
-
-	case LOCATE_TAG_ANY_TYPE:
-		return std::string("(" LOCATE_TAG_ANY_KEY " ") + (IsNegated() ? "!=" : "==") + " \"" + string_filter.GetValue() + "\")";
-
-	default:
-		return std::string("(") + tag_item_names[tag] + " " + (IsNegated() ? "!=" : "==") + " \"" + string_filter.GetValue() + "\")";
-	}
 }
 
 bool
-SongFilter::Item::MatchNN(const TagItem &item) const noexcept
+BaseSongFilter::Match(const LightSong &song) const noexcept
 {
-	return (tag == LOCATE_TAG_ANY_TYPE || (unsigned)item.type == tag) &&
+	return uri_is_child_or_same(value.c_str(), song.GetURI().c_str());
-		string_filter.Match(item.value);
+}
+
+std::string
+TagSongFilter::ToExpression() const noexcept
+{
+	const char *name = type == TAG_NUM_OF_ITEM_TYPES
+		? LOCATE_TAG_ANY_KEY
+		: tag_item_names[type];
+
+	return std::string("(") + name + " " + (negated ? "!=" : "==") + " \"" + filter.GetValue() + "\")";
 }
 
 bool
-SongFilter::Item::MatchNN(const Tag &_tag) const noexcept
+TagSongFilter::MatchNN(const TagItem &item) const noexcept
+{
+	return (type == TAG_NUM_OF_ITEM_TYPES || item.type == type) &&
+		filter.Match(item.value);
+}
+
+bool
+TagSongFilter::MatchNN(const Tag &tag) const noexcept
 {
 	bool visited_types[TAG_NUM_OF_ITEM_TYPES];
 	std::fill_n(visited_types, size_t(TAG_NUM_OF_ITEM_TYPES), false);
 
-	for (const auto &i : _tag) {
+	for (const auto &i : tag) {
 		visited_types[i.type] = true;
 
 		if (MatchNN(i))
 			return true;
 	}
 
-	if (tag < TAG_NUM_OF_ITEM_TYPES && !visited_types[tag]) {
+	if (type < TAG_NUM_OF_ITEM_TYPES && !visited_types[type]) {
 		/* If the search critieron was not visited during the
 		   sweep through the song's tag, it means this field
 		   is absent from the tag or empty. Thus, if the
 		   searched string is also empty
 		   then it's a match as well and we should return
 		   true. */
-		if (string_filter.empty())
+		if (filter.empty())
 			return true;
 
-		if (tag == TAG_ALBUM_ARTIST && visited_types[TAG_ARTIST]) {
+		if (type == TAG_ALBUM_ARTIST && visited_types[TAG_ARTIST]) {
 			/* if we're looking for "album artist", but
 			   only "artist" exists, use that */
-			for (const auto &item : _tag)
+			for (const auto &item : tag)
 				if (item.type == TAG_ARTIST &&
-				    string_filter.Match(item.value))
+				    filter.Match(item.value))
 					return true;
 		}
 	}
@@ -154,28 +156,27 @@ SongFilter::Item::MatchNN(const Tag &_tag) const noexcept
 }
 
 bool
-SongFilter::Item::MatchNN(const LightSong &song) const noexcept
+TagSongFilter::Match(const LightSong &song) const noexcept
 {
-	if (tag == LOCATE_TAG_BASE_TYPE) {
+	return MatchNN(song.tag) != negated;
-		const auto uri = song.GetURI();
-		return uri_is_child_or_same(string_filter.GetValue().c_str(),
-					    uri.c_str());
-	}
-
-	if (tag == LOCATE_TAG_MODIFIED_SINCE)
-		return song.mtime >= time;
-
-	if (tag == LOCATE_TAG_FILE_TYPE) {
-		const auto uri = song.GetURI();
-		return string_filter.Match(uri.c_str());
-	}
-
-	return MatchNN(song.tag);
 }
 
-SongFilter::SongFilter(unsigned tag, const char *value, bool fold_case)
+std::string
+ModifiedSinceSongFilter::ToExpression() const noexcept
 {
-	items.emplace_back(tag, value, fold_case);
+	return std::string("(modified-since \"") + FormatISO8601(value).c_str() + "\")";
+}
+
+bool
+ModifiedSinceSongFilter::Match(const LightSong &song) const noexcept
+{
+	return song.mtime >= value;
+}
+
+SongFilter::SongFilter(TagType tag, const char *value, bool fold_case)
+{
+	items.emplace_back(std::make_unique<TagSongFilter>(tag, value,
+							   fold_case, false));
 }
 
 SongFilter::~SongFilter()
@@ -190,14 +191,14 @@ SongFilter::ToExpression() const noexcept
 	const auto end = items.end();
 
 	if (std::next(i) == end)
-		return i->ToExpression();
+		return (*i)->ToExpression();
 
 	std::string e("(");
-	e += i->ToExpression();
+	e += (*i)->ToExpression();
 
 	for (++i; i != end; ++i) {
 		e += " AND ";
-		e += i->ToExpression();
+		e += (*i)->ToExpression();
 	}
 
 	e.push_back(')');
@@ -273,8 +274,8 @@ ExpectQuoted(const char *&s)
 	return {begin, end};
 }
 
-const char *
+ISongFilterPtr
-SongFilter::ParseExpression(const char *s, bool fold_case)
+SongFilter::ParseExpression(const char *&s, bool fold_case)
 {
 	assert(*s == '(');
 
@@ -283,21 +284,21 @@ SongFilter::ParseExpression(const char *s, bool fold_case)
 	if (*s == '(')
 		throw std::runtime_error("Nested expressions not yet implemented");
 
-	const auto type = ExpectFilterType(s);
+	auto type = ExpectFilterType(s);
 
 	if (type == LOCATE_TAG_MODIFIED_SINCE) {
 		const auto value_s = ExpectQuoted(s);
 		if (*s != ')')
 			throw std::runtime_error("')' expected");
-		items.emplace_back(type, ParseTimeStamp(value_s.c_str()));
+		s = StripLeft(s + 1);
-		return StripLeft(s + 1);
+		return std::make_unique<ModifiedSinceSongFilter>(ParseTimeStamp(value_s.c_str()));
 	} else if (type == LOCATE_TAG_BASE_TYPE) {
 		auto value = ExpectQuoted(s);
 		if (*s != ')')
 			throw std::runtime_error("')' expected");
+		s = StripLeft(s + 1);
 
-		items.emplace_back(type, std::move(value), fold_case);
+		return std::make_unique<BaseSongFilter>(std::move(value));
-		return StripLeft(s + 1);
 	} else {
 		bool negated = false;
 		if (s[0] == '!' && s[1] == '=')
@@ -310,9 +311,19 @@ SongFilter::ParseExpression(const char *s, bool fold_case)
 		if (*s != ')')
 			throw std::runtime_error("')' expected");
 
-		items.emplace_back(type, std::move(value), fold_case);
+		s = StripLeft(s + 1);
-		items.back().SetNegated(negated);
+
-		return StripLeft(s + 1);
+		if (type == LOCATE_TAG_ANY_TYPE)
+			type = TAG_NUM_OF_ITEM_TYPES;
+
+		if (type == LOCATE_TAG_FILE_TYPE)
+			return std::make_unique<UriSongFilter>(std::move(value),
+							       fold_case,
+							       negated);
+
+		return std::make_unique<TagSongFilter>(TagType(type),
+						       std::move(value),
+						       fold_case, negated);
 	}
 }
 
@@ -320,21 +331,38 @@ void
 SongFilter::Parse(const char *tag_string, const char *value, bool fold_case)
 {
 	unsigned tag = locate_parse_type(tag_string);
-	if (tag == TAG_NUM_OF_ITEM_TYPES)
+
+	switch (tag) {
+	case TAG_NUM_OF_ITEM_TYPES:
 		throw std::runtime_error("Unknown filter type");
 
-	if (tag == LOCATE_TAG_BASE_TYPE) {
+	case LOCATE_TAG_BASE_TYPE:
 		if (!uri_safe_local(value))
 			throw std::runtime_error("Bad URI");
 
-		/* case folding doesn't work with "base" */
+		items.emplace_back(std::make_unique<BaseSongFilter>(value));
-		fold_case = false;
+		break;
-	}
 
-	if (tag == LOCATE_TAG_MODIFIED_SINCE)
+	case LOCATE_TAG_MODIFIED_SINCE:
-		items.emplace_back(tag, ParseTimeStamp(value));
+		items.emplace_back(std::make_unique<ModifiedSinceSongFilter>(ParseTimeStamp(value)));
-	else
+		break;
-		items.emplace_back(tag, value, fold_case);
+
+	case LOCATE_TAG_FILE_TYPE:
+		items.emplace_back(std::make_unique<UriSongFilter>(value,
+								   fold_case,
+								   false));
+		break;
+
+	default:
+		if (tag == LOCATE_TAG_ANY_TYPE)
+			tag = TAG_NUM_OF_ITEM_TYPES;
+
+		items.emplace_back(std::make_unique<TagSongFilter>(TagType(tag),
+								   value,
+								   fold_case,
+								   false));
+		break;
+	}
 }
 
 void
@@ -346,10 +374,12 @@ SongFilter::Parse(ConstBuffer<const char *> args, bool fold_case)
 	do {
 		if (*args.front() == '(') {
 			const char *s = args.shift();
-			const char *end = ParseExpression(s, fold_case);
+			const char *end = s;
+			auto f = ParseExpression(end, fold_case);
 			if (*end != 0)
 				throw std::runtime_error("Unparsed garbage after expression");
 
+			items.emplace_back(std::move(f));
 			continue;
 		}
 
@@ -366,18 +396,36 @@ bool
 SongFilter::Match(const LightSong &song) const noexcept
 {
 	for (const auto &i : items)
-		if (!i.Match(song))
+		if (!i->Match(song))
 			return false;
 
 	return true;
 }
 
+bool
+SongFilter::HasFoldCase() const noexcept
+{
+	for (const auto &i : items) {
+		if (auto t = dynamic_cast<const TagSongFilter *>(i.get())) {
+			if (t->GetFoldCase())
+				return true;
+		} else if (auto u = dynamic_cast<const UriSongFilter *>(i.get())) {
+			if (u->GetFoldCase())
+				return true;
+		}
+	}
+
+	return false;
+}
+
 bool
 SongFilter::HasOtherThanBase() const noexcept
 {
-	for (const auto &i : items)
+	for (const auto &i : items) {
-		if (i.GetTag() != LOCATE_TAG_BASE_TYPE)
+		const auto *f = dynamic_cast<const BaseSongFilter *>(i.get());
+		if (f == nullptr)
 			return true;
+	}
 
 	return false;
 }
@@ -385,9 +433,11 @@ SongFilter::HasOtherThanBase() const noexcept
 const char *
 SongFilter::GetBase() const noexcept
 {
-	for (const auto &i : items)
+	for (const auto &i : items) {
-		if (i.GetTag() == LOCATE_TAG_BASE_TYPE)
+		const auto *f = dynamic_cast<const BaseSongFilter *>(i.get());
-			return i.GetValue();
+		if (f != nullptr)
+			return f->GetValue();
+	}
 
 	return nullptr;
 }
@@ -399,8 +449,9 @@ SongFilter::WithoutBasePrefix(const char *_prefix) const noexcept
 	SongFilter result;
 
 	for (const auto &i : items) {
-		if (i.GetTag() == LOCATE_TAG_BASE_TYPE) {
+		const auto *f = dynamic_cast<const BaseSongFilter *>(i.get());
-			const char *s = StringAfterPrefix(i.GetValue(), prefix);
+		if (f != nullptr) {
+			const char *s = StringAfterPrefix(f->GetValue(), prefix);
 			if (s != nullptr) {
 				if (*s == 0)
 					continue;
@@ -409,14 +460,14 @@ SongFilter::WithoutBasePrefix(const char *_prefix) const noexcept
 					++s;
 
 					if (*s != 0)
-						result.items.emplace_back(LOCATE_TAG_BASE_TYPE, s);
+						result.items.emplace_back(std::make_unique<BaseSongFilter>(s));
 
 					continue;
 				}
 			}
 		}
 
-		result.items.emplace_back(i);
+		result.items.emplace_back(i->Clone());
 	}
 
 	return result;

src/SongFilter.hxx

@@ -1,5 +1,5 @@
 /*
- * Copyright 2003-2017 The Music Player Daemon Project
+ * Copyright 2003-2018 The Music Player Daemon Project
  * http://www.musicpd.org
  *
  * This program is free software; you can redistribute it and/or modify
@@ -23,10 +23,13 @@
 #include "lib/icu/Compare.hxx"
 #include "Compiler.h"
 
+#include <memory>
 #include <string>
 #include <list>
 #include <chrono>
 
+#include <stdint.h>
+
 /**
  * Limit the search to files within the given directory.
  */
@@ -42,9 +45,28 @@
 #define LOCATE_TAG_ANY_TYPE     TAG_NUM_OF_ITEM_TYPES+20
 
 template<typename T> struct ConstBuffer;
+enum TagType : uint8_t;
 struct Tag;
 struct TagItem;
 struct LightSong;
+class ISongFilter;
+using ISongFilterPtr = std::unique_ptr<ISongFilter>;
+
+class ISongFilter {
+public:
+	virtual ~ISongFilter() noexcept {}
+
+	virtual ISongFilterPtr Clone() const noexcept = 0;
+
+	/**
+	 * Convert this object into an "expression".  This is
+	 * only useful for debugging.
+	 */
+	virtual std::string ToExpression() const noexcept = 0;
+
+	gcc_pure
+	virtual bool Match(const LightSong &song) const noexcept = 0;
+};
 
 class StringFilter {
 	std::string value;
@@ -55,8 +77,6 @@ class StringFilter {
 	IcuCompare fold_case;
 
 public:
-	StringFilter() = default;
-
 	template<typename V>
 	StringFilter(V &&_value, bool _fold_case)
 		:value(std::forward<V>(_value)),
@@ -80,82 +100,129 @@ public:
 	bool Match(const char *s) const noexcept;
 };
 
-class SongFilter {
+class UriSongFilter final : public ISongFilter {
+	StringFilter filter;
+
+	bool negated;
+
 public:
-	class Item {
+	template<typename V>
-		unsigned tag;
+	UriSongFilter(V &&_value, bool fold_case, bool _negated)
+		:filter(std::forward<V>(_value), fold_case),
+		 negated(_negated) {}
 
-		bool negated = false;
+	const auto &GetValue() const noexcept {
+		return filter.GetValue();
+	}
 
-		StringFilter string_filter;
+	bool GetFoldCase() const {
+		return filter.GetFoldCase();
+	}
 
-		/**
+	bool IsNegated() const noexcept {
-		 * For #LOCATE_TAG_MODIFIED_SINCE
+		return negated;
-		 */
+	}
-		std::chrono::system_clock::time_point time;
 
-	public:
+	ISongFilterPtr Clone() const noexcept override {
-		Item(unsigned tag, std::string &&_value, bool fold_case=false);
+		return std::make_unique<UriSongFilter>(*this);
-		Item(unsigned tag, std::chrono::system_clock::time_point time);
+	}
 
-		/**
+	std::string ToExpression() const noexcept override;
-		 * Convert this object into an "expression".  This is
+	bool Match(const LightSong &song) const noexcept override;
-		 * only useful for debugging.
+};
-		 */
-		std::string ToExpression() const noexcept;
 
-		unsigned GetTag() const {
+class BaseSongFilter final : public ISongFilter {
-			return tag;
+	std::string value;
-		}
 
-		bool IsNegated() const noexcept {
+public:
-			return negated;
+	BaseSongFilter(const BaseSongFilter &) = default;
-		}
 
-		void SetNegated(bool _negated=true) noexcept {
+	template<typename V>
-			negated = _negated;
+	explicit BaseSongFilter(V &&_value)
-		}
+		:value(std::forward<V>(_value)) {}
 
-		bool GetFoldCase() const {
+	const char *GetValue() const noexcept {
-			return string_filter.GetFoldCase();
+		return value.c_str();
-		}
+	}
 
-		const char *GetValue() const {
+	ISongFilterPtr Clone() const noexcept override {
-			return string_filter.GetValue().c_str();
+		return std::make_unique<BaseSongFilter>(*this);
-		}
+	}
 
-	private:
+	std::string ToExpression() const noexcept override;
-		/* note: the "NN" suffix means "no negation", i.e. the
+	bool Match(const LightSong &song) const noexcept override;
-		   method pretends negation is unset, and the caller
+};
-		   is responsibly for considering it */
 
-		gcc_pure
+class TagSongFilter final : public ISongFilter {
-		bool MatchNN(const TagItem &tag_item) const noexcept;
+	TagType type;
 
-		gcc_pure
+	bool negated;
-		bool MatchNN(const Tag &tag) const noexcept;
 
-		gcc_pure
+	StringFilter filter;
-		bool MatchNN(const LightSong &song) const noexcept;
 
-	public:
+public:
-		gcc_pure
+	template<typename V>
-		bool Match(const LightSong &song) const noexcept {
+	TagSongFilter(TagType _type, V &&_value, bool fold_case, bool _negated)
-			return MatchNN(song) != IsNegated();
+		:type(_type), negated(_negated),
-		}
+		 filter(std::forward<V>(_value), fold_case) {}
-	};
+
+	TagType GetTagType() const {
+		return type;
+	}
+
+	const auto &GetValue() const noexcept {
+		return filter.GetValue();
+	}
+
+	bool GetFoldCase() const {
+		return filter.GetFoldCase();
+	}
+
+	bool IsNegated() const noexcept {
+		return negated;
+	}
+
+	ISongFilterPtr Clone() const noexcept override {
+		return std::make_unique<TagSongFilter>(*this);
+	}
+
+	std::string ToExpression() const noexcept override;
+	bool Match(const LightSong &song) const noexcept override;
 
 private:
-	std::list<Item> items;
+	bool MatchNN(const Tag &tag) const noexcept;
+	bool MatchNN(const TagItem &tag) const noexcept;
+};
+
+class ModifiedSinceSongFilter final : public ISongFilter {
+	std::chrono::system_clock::time_point value;
+
+public:
+	explicit ModifiedSinceSongFilter(std::chrono::system_clock::time_point _value) noexcept
+		:value(_value) {}
+
+	ISongFilterPtr Clone() const noexcept override {
+		return std::make_unique<ModifiedSinceSongFilter>(*this);
+	}
+
+	std::string ToExpression() const noexcept override;
+	bool Match(const LightSong &song) const noexcept override;
+};
+
+class SongFilter {
+	std::list<ISongFilterPtr> items;
 
 public:
 	SongFilter() = default;
 
 	gcc_nonnull(3)
-	SongFilter(unsigned tag, const char *value, bool fold_case=false);
+	SongFilter(TagType tag, const char *value, bool fold_case=false);
 
 	~SongFilter();
 
+	SongFilter(SongFilter &&) = default;
+	SongFilter &operator=(SongFilter &&) = default;
+
 	/**
 	 * Convert this object into an "expression".  This is
 	 * only useful for debugging.
@@ -163,7 +230,7 @@ public:
 	std::string ToExpression() const noexcept;
 
 private:
-	const char *ParseExpression(const char *s, bool fold_case=false);
+	ISongFilterPtr ParseExpression(const char *&s, bool fold_case=false);
 
 	gcc_nonnull(2,3)
 	void Parse(const char *tag, const char *value, bool fold_case=false);
@@ -177,7 +244,7 @@ public:
 	gcc_pure
 	bool Match(const LightSong &song) const noexcept;
 
-	const std::list<Item> &GetItems() const noexcept {
+	const auto &GetItems() const noexcept {
 		return items;
 	}
 
@@ -190,13 +257,7 @@ public:
 	 * Is there at least one item with "fold case" enabled?
 	 */
 	gcc_pure
-	bool HasFoldCase() const noexcept {
+	bool HasFoldCase() const noexcept;
-		for (const auto &i : items)
-			if (i.GetFoldCase())
-				return true;
-
-		return false;
-	}
 
 	/**
 	 * Does this filter contain constraints other than "base"?

src/command/DatabaseCommands.cxx

@@ -248,7 +248,7 @@ handle_list(Client &client, Request args, Response &r)
 			return CommandResult::ERROR;
 		}
 
-		filter.reset(new SongFilter((unsigned)TAG_ARTIST,
+		filter.reset(new SongFilter(TAG_ARTIST,
 					    args.shift()));
 	}
 

src/db/plugins/ProxyDatabasePlugin.cxx

@@ -268,49 +268,53 @@ CheckError(struct mpd_connection *connection)
 }
 
 static bool
-SendConstraints(mpd_connection *connection, const SongFilter::Item &item)
+SendConstraints(mpd_connection *connection, const ISongFilter &f)
 {
-	switch (item.GetTag()) {
+	if (auto t = dynamic_cast<const TagSongFilter *>(&f)) {
-		mpd_tag_type tag;
+		if (t->IsNegated())
-
+			// TODO implement
-#if LIBMPDCLIENT_CHECK_VERSION(2,9,0)
-	case LOCATE_TAG_BASE_TYPE:
-		if (mpd_connection_cmp_server_version(connection, 0, 18, 0) < 0)
-			/* requires MPD 0.18 */
 			return true;
 
-		return mpd_search_add_base_constraint(connection,
+		if (t->GetTagType() == TAG_NUM_OF_ITEM_TYPES)
-						      MPD_OPERATOR_DEFAULT,
+			return mpd_search_add_any_tag_constraint(connection,
-						      item.GetValue());
+								 MPD_OPERATOR_DEFAULT,
-#endif
+								 t->GetValue().c_str());
 
-	case LOCATE_TAG_FILE_TYPE:
+		const auto tag = Convert(t->GetTagType());
-		return mpd_search_add_uri_constraint(connection,
-						     MPD_OPERATOR_DEFAULT,
-						     item.GetValue());
-
-	case LOCATE_TAG_ANY_TYPE:
-		return mpd_search_add_any_tag_constraint(connection,
-							 MPD_OPERATOR_DEFAULT,
-							 item.GetValue());
-
-	default:
-		tag = Convert(TagType(item.GetTag()));
 		if (tag == MPD_TAG_COUNT)
 			return true;
 
 		return mpd_search_add_tag_constraint(connection,
 						     MPD_OPERATOR_DEFAULT,
 						     tag,
-						     item.GetValue());
+						     t->GetValue().c_str());
-	}
+	} else if (auto u = dynamic_cast<const UriSongFilter *>(&f)) {
+		if (u->IsNegated())
+			// TODO implement
+			return true;
+
+		return mpd_search_add_uri_constraint(connection,
+						     MPD_OPERATOR_DEFAULT,
+						     u->GetValue().c_str());
+#if LIBMPDCLIENT_CHECK_VERSION(2,9,0)
+	} else if (auto b = dynamic_cast<const BaseSongFilter *>(&f)) {
+		if (mpd_connection_cmp_server_version(connection, 0, 18, 0) < 0)
+			/* requires MPD 0.18 */
+			return true;
+
+		return mpd_search_add_base_constraint(connection,
+						      MPD_OPERATOR_DEFAULT,
+						      b->GetValue());
+#endif
+	} else
+		return true;
 }
 
 static bool
 SendConstraints(mpd_connection *connection, const SongFilter &filter)
 {
 	for (const auto &i : filter.GetItems())
-		if (!SendConstraints(connection, i))
+		if (!SendConstraints(connection, *i))
 			return false;
 
 	return true;

src/db/plugins/upnp/UpnpDatabasePlugin.cxx

@@ -254,9 +254,9 @@ UpnpDatabase::SearchSongs(const ContentDirectoryService &server,
 
 	std::string cond;
 	for (const auto &item : filter->GetItems()) {
-		switch (auto tag = item.GetTag()) {
+		if (auto t = dynamic_cast<const TagSongFilter *>(item.get())) {
-		case LOCATE_TAG_ANY_TYPE:
+			auto tag = t->GetTagType();
-			{
+			if (tag == TAG_NUM_OF_ITEM_TYPES) {
 				if (!cond.empty()) {
 					cond += " and ";
 				}
@@ -268,29 +268,21 @@ UpnpDatabase::SearchSongs(const ContentDirectoryService &server,
 					else
 						cond += " or ";
 					cond += cap;
-					if (item.GetFoldCase()) {
+					if (t->GetFoldCase()) {
 						cond += " contains ";
 					} else {
 						cond += " = ";
 					}
-					dquote(cond, item.GetValue());
+					dquote(cond, t->GetValue().c_str());
 				}
 				cond += ')';
+				continue;
 			}
-			break;
 
-		default:
-			/* Unhandled conditions like
-			   LOCATE_TAG_BASE_TYPE or
-			   LOCATE_TAG_FILE_TYPE won't have a
-			   corresponding upnp prop, so they will be
-			   skipped */
 			if (tag == TAG_ALBUM_ARTIST)
 				tag = TAG_ARTIST;
 
-			// TODO: support LOCATE_TAG_ANY_TYPE etc.
+			const char *name = tag_table_lookup(upnp_tags, tag);
-			const char *name = tag_table_lookup(upnp_tags,
-							    TagType(tag));
 			if (name == nullptr)
 				continue;
 
@@ -304,13 +296,15 @@ UpnpDatabase::SearchSongs(const ContentDirectoryService &server,
 			   case-insensitive, but at least some servers
 			   have the same convention as mpd (e.g.:
 			   minidlna) */
-			if (item.GetFoldCase()) {
+			if (t->GetFoldCase()) {
 				cond += " contains ";
 			} else {
 				cond += " = ";
 			}
-			dquote(cond, item.GetValue());
+			dquote(cond, t->GetValue().c_str());
 		}
+
+		// TODO: support other ISongFilter implementations
 	}
 
 	return server.search(handle, objid, cond.c_str());