From 2fd51826087897f2ce5bfd9ca27c1e167a3e4284 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 19 Mar 2016 00:13:57 +0100 Subject: [PATCH] db/Interface: GetSong() throws exception on error --- src/PlaylistPrint.cxx | 4 +--- src/SongLoader.cxx | 20 ++++++++----------- src/SongLoader.hxx | 7 +++---- src/command/QueueCommands.cxx | 14 ++++++------- src/command/StickerCommands.cxx | 19 +++++++----------- src/db/DatabaseSong.cxx | 8 +++----- src/db/DatabaseSong.hxx | 5 ++--- src/db/Interface.hxx | 3 +-- src/db/plugins/ProxyDatabasePlugin.cxx | 5 ++--- .../plugins/simple/SimpleDatabasePlugin.cxx | 4 ++-- .../plugins/simple/SimpleDatabasePlugin.hxx | 3 +-- src/db/plugins/upnp/UpnpDatabasePlugin.cxx | 5 ++--- src/queue/PlaylistUpdate.cxx | 2 +- src/sticker/SongSticker.cxx | 8 +++----- test/test_translate_song.cxx | 3 +-- 15 files changed, 43 insertions(+), 67 deletions(-) diff --git a/src/PlaylistPrint.cxx b/src/PlaylistPrint.cxx index e04d2cab9..ccab06527 100644 --- a/src/PlaylistPrint.cxx +++ b/src/PlaylistPrint.cxx @@ -127,9 +127,7 @@ PrintSongDetails(Response &r, Partition &partition, const char *uri_utf8) const LightSong *song; try { - song = db->GetSong(uri_utf8, IgnoreError()); - if (song == nullptr) - return false; + song = db->GetSong(uri_utf8); } catch (const std::runtime_error &e) { return false; } diff --git a/src/SongLoader.cxx b/src/SongLoader.cxx index 568047411..f2dae665a 100644 --- a/src/SongLoader.cxx +++ b/src/SongLoader.cxx @@ -38,21 +38,20 @@ SongLoader::SongLoader(const Client &_client) #endif DetachedSong * -SongLoader::LoadFromDatabase(const char *uri, Error &error) const +SongLoader::LoadFromDatabase(const char *uri) const { #ifdef ENABLE_DATABASE if (db != nullptr) - return DatabaseDetachSong(*db, *storage, uri, error); + return DatabaseDetachSong(*db, *storage, uri); #else (void)uri; - (void)error; #endif throw PlaylistError(PlaylistResult::NO_SUCH_SONG, "No database"); } DetachedSong * -SongLoader::LoadFile(const char *path_utf8, Path path_fs, Error &error) const +SongLoader::LoadFile(const char *path_utf8, Path path_fs) const { #ifdef ENABLE_DATABASE if (storage != nullptr) { @@ -60,10 +59,8 @@ SongLoader::LoadFile(const char *path_utf8, Path path_fs, Error &error) const if (suffix != nullptr) /* this path was relative to the music directory - obtain it from the database */ - return LoadFromDatabase(suffix, error); + return LoadFromDatabase(suffix); } -#else - (void)error; #endif DetachedSong song(path_utf8); @@ -74,7 +71,7 @@ SongLoader::LoadFile(const char *path_utf8, Path path_fs, Error &error) const } DetachedSong * -SongLoader::LoadSong(const LocatedUri &located_uri, Error &error) const +SongLoader::LoadSong(const LocatedUri &located_uri) const { switch (located_uri.type) { case LocatedUri::Type::UNKNOWN: @@ -84,11 +81,10 @@ SongLoader::LoadSong(const LocatedUri &located_uri, Error &error) const return new DetachedSong(located_uri.canonical_uri); case LocatedUri::Type::RELATIVE: - return LoadFromDatabase(located_uri.canonical_uri, error); + return LoadFromDatabase(located_uri.canonical_uri); case LocatedUri::Type::PATH: - return LoadFile(located_uri.canonical_uri, located_uri.path, - error); + return LoadFile(located_uri.canonical_uri, located_uri.path); } gcc_unreachable(); @@ -110,5 +106,5 @@ SongLoader::LoadSong(const char *uri_utf8, Error &error) const if (located_uri.IsUnknown()) return nullptr; - return LoadSong(located_uri, error); + return LoadSong(located_uri); } diff --git a/src/SongLoader.hxx b/src/SongLoader.hxx index f86a89678..392849ce8 100644 --- a/src/SongLoader.hxx +++ b/src/SongLoader.hxx @@ -68,18 +68,17 @@ public: } #endif - DetachedSong *LoadSong(const LocatedUri &uri, Error &error) const; + DetachedSong *LoadSong(const LocatedUri &uri) const; gcc_nonnull_all DetachedSong *LoadSong(const char *uri_utf8, Error &error) const; private: gcc_nonnull_all - DetachedSong *LoadFromDatabase(const char *uri, Error &error) const; + DetachedSong *LoadFromDatabase(const char *uri) const; gcc_nonnull_all - DetachedSong *LoadFile(const char *path_utf8, Path path_fs, - Error &error) const; + DetachedSong *LoadFile(const char *path_utf8, Path path_fs) const; }; #endif diff --git a/src/command/QueueCommands.cxx b/src/command/QueueCommands.cxx index 5505bc329..fc0d244e6 100644 --- a/src/command/QueueCommands.cxx +++ b/src/command/QueueCommands.cxx @@ -41,17 +41,14 @@ #include #include -static CommandResult -AddUri(Client &client, const LocatedUri &uri, Response &r) +static void +AddUri(Client &client, const LocatedUri &uri) { - Error error; - std::unique_ptr song(SongLoader(client).LoadSong(uri, error)); - if (song == nullptr) - return print_error(r, error); + std::unique_ptr song(SongLoader(client).LoadSong(uri)); + assert(song); auto &partition = client.partition; partition.playlist.AppendSong(partition.pc, std::move(*song)); - return CommandResult::OK; } static CommandResult @@ -98,7 +95,8 @@ handle_add(Client &client, Request args, Response &r) case LocatedUri::Type::ABSOLUTE: case LocatedUri::Type::PATH: - return AddUri(client, located_uri, r); + AddUri(client, located_uri); + return CommandResult::OK; case LocatedUri::Type::RELATIVE: return AddDatabaseSelection(client, located_uri.canonical_uri, diff --git a/src/command/StickerCommands.cxx b/src/command/StickerCommands.cxx index 7fea154e0..5c4de3292 100644 --- a/src/command/StickerCommands.cxx +++ b/src/command/StickerCommands.cxx @@ -62,9 +62,7 @@ handle_sticker_song(Response &r, Partition &partition, Request args) /* get song song_id key */ if (args.size == 4 && StringIsEqual(cmd, "get")) { - const LightSong *song = db->GetSong(args[2], error); - if (song == nullptr) - return print_error(r, error); + const LightSong *song = db->GetSong(args[2]); const auto value = sticker_song_get_value(*song, args[3], error); @@ -82,9 +80,8 @@ handle_sticker_song(Response &r, Partition &partition, Request args) return CommandResult::OK; /* list song song_id */ } else if (args.size == 3 && StringIsEqual(cmd, "list")) { - const LightSong *song = db->GetSong(args[2], error); - if (song == nullptr) - return print_error(r, error); + const LightSong *song = db->GetSong(args[2]); + assert(song != nullptr); Sticker *sticker = sticker_song_get(*song, error); db->ReturnSong(song); @@ -97,9 +94,8 @@ handle_sticker_song(Response &r, Partition &partition, Request args) return CommandResult::OK; /* set song song_id id key */ } else if (args.size == 5 && StringIsEqual(cmd, "set")) { - const LightSong *song = db->GetSong(args[2], error); - if (song == nullptr) - return print_error(r, error); + const LightSong *song = db->GetSong(args[2]); + assert(song != nullptr); bool ret = sticker_song_set_value(*song, args[3], args[4], error); @@ -117,9 +113,8 @@ handle_sticker_song(Response &r, Partition &partition, Request args) /* delete song song_id [key] */ } else if ((args.size == 3 || args.size == 4) && StringIsEqual(cmd, "delete")) { - const LightSong *song = db->GetSong(args[2], error); - if (song == nullptr) - return print_error(r, error); + const LightSong *song = db->GetSong(args[2]); + assert(song != nullptr); bool ret = args.size == 3 ? sticker_song_delete(*song, error) diff --git a/src/db/DatabaseSong.cxx b/src/db/DatabaseSong.cxx index ec4f6e34e..9f3191dbf 100644 --- a/src/db/DatabaseSong.cxx +++ b/src/db/DatabaseSong.cxx @@ -41,12 +41,10 @@ DatabaseDetachSong(const Storage &storage, const LightSong &song) } DetachedSong * -DatabaseDetachSong(const Database &db, const Storage &storage, const char *uri, - Error &error) +DatabaseDetachSong(const Database &db, const Storage &storage, const char *uri) { - const LightSong *tmp = db.GetSong(uri, error); - if (tmp == nullptr) - return nullptr; + const LightSong *tmp = db.GetSong(uri); + assert(tmp != nullptr); DetachedSong *song = new DetachedSong(DatabaseDetachSong(storage, *tmp)); diff --git a/src/db/DatabaseSong.hxx b/src/db/DatabaseSong.hxx index c60e385cb..615603c9f 100644 --- a/src/db/DatabaseSong.hxx +++ b/src/db/DatabaseSong.hxx @@ -26,7 +26,6 @@ struct LightSong; class Database; class Storage; class DetachedSong; -class Error; /** * "Detach" the #Song object, i.e. convert it to a #DetachedSong @@ -44,7 +43,7 @@ DatabaseDetachSong(const Storage &storage, const LightSong &song); */ gcc_malloc gcc_nonnull_all DetachedSong * -DatabaseDetachSong(const Database &db, const Storage &storage, const char *uri, - Error &error); +DatabaseDetachSong(const Database &db, const Storage &storage, + const char *uri); #endif diff --git a/src/db/Interface.hxx b/src/db/Interface.hxx index e0c29395c..625328b42 100644 --- a/src/db/Interface.hxx +++ b/src/db/Interface.hxx @@ -73,8 +73,7 @@ public: * @param uri_utf8 the URI of the song within the music * directory (UTF-8) */ - virtual const LightSong *GetSong(const char *uri_utf8, - Error &error) const = 0; + virtual const LightSong *GetSong(const char *uri_utf8) const = 0; /** * Mark the song object as "unused". Call this on objects diff --git a/src/db/plugins/ProxyDatabasePlugin.cxx b/src/db/plugins/ProxyDatabasePlugin.cxx index 9a9f3e2f1..57047083c 100644 --- a/src/db/plugins/ProxyDatabasePlugin.cxx +++ b/src/db/plugins/ProxyDatabasePlugin.cxx @@ -115,8 +115,7 @@ public: virtual void Open() override; virtual void Close() override; - virtual const LightSong *GetSong(const char *uri_utf8, - Error &error) const override; + const LightSong *GetSong(const char *uri_utf8) const override; void ReturnSong(const LightSong *song) const override; virtual bool Visit(const DatabaseSelection &selection, @@ -519,7 +518,7 @@ ProxyDatabase::OnIdle() } const LightSong * -ProxyDatabase::GetSong(const char *uri, gcc_unused Error &error) const +ProxyDatabase::GetSong(const char *uri) const { // TODO: eliminate the const_cast const_cast(this)->EnsureConnected(); diff --git a/src/db/plugins/simple/SimpleDatabasePlugin.cxx b/src/db/plugins/simple/SimpleDatabasePlugin.cxx index b10f9e8ee..84328b998 100644 --- a/src/db/plugins/simple/SimpleDatabasePlugin.cxx +++ b/src/db/plugins/simple/SimpleDatabasePlugin.cxx @@ -229,7 +229,7 @@ SimpleDatabase::Close() } const LightSong * -SimpleDatabase::GetSong(const char *uri, Error &error) const +SimpleDatabase::GetSong(const char *uri) const { assert(root != nullptr); assert(prefixed_light_song == nullptr); @@ -244,7 +244,7 @@ SimpleDatabase::GetSong(const char *uri, Error &error) const protect.unlock(); const LightSong *song = - r.directory->mounted_database->GetSong(r.uri, error); + r.directory->mounted_database->GetSong(r.uri); if (song == nullptr) return nullptr; diff --git a/src/db/plugins/simple/SimpleDatabasePlugin.hxx b/src/db/plugins/simple/SimpleDatabasePlugin.hxx index c0ecdbd13..f98486b6f 100644 --- a/src/db/plugins/simple/SimpleDatabasePlugin.hxx +++ b/src/db/plugins/simple/SimpleDatabasePlugin.hxx @@ -110,8 +110,7 @@ public: virtual void Open() override; virtual void Close() override; - const LightSong *GetSong(const char *uri_utf8, - Error &error) const override; + const LightSong *GetSong(const char *uri_utf8) const override; void ReturnSong(const LightSong *song) const override; virtual bool Visit(const DatabaseSelection &selection, diff --git a/src/db/plugins/upnp/UpnpDatabasePlugin.cxx b/src/db/plugins/upnp/UpnpDatabasePlugin.cxx index 2e23b206c..f8d06238b 100644 --- a/src/db/plugins/upnp/UpnpDatabasePlugin.cxx +++ b/src/db/plugins/upnp/UpnpDatabasePlugin.cxx @@ -82,8 +82,7 @@ public: virtual void Open() override; virtual void Close() override; - virtual const LightSong *GetSong(const char *uri_utf8, - Error &error) const override; + virtual const LightSong *GetSong(const char *uri_utf8) const override; void ReturnSong(const LightSong *song) const override; virtual bool Visit(const DatabaseSelection &selection, @@ -202,7 +201,7 @@ UpnpDatabase::ReturnSong(const LightSong *_song) const // Get song info by path. We can receive either the id path, or the titles // one const LightSong * -UpnpDatabase::GetSong(const char *uri, gcc_unused Error &error) const +UpnpDatabase::GetSong(const char *uri) const { auto vpath = stringToTokens(uri, "/", true); if (vpath.size() < 2) diff --git a/src/queue/PlaylistUpdate.cxx b/src/queue/PlaylistUpdate.cxx index 13d4f4de6..208b15222 100644 --- a/src/queue/PlaylistUpdate.cxx +++ b/src/queue/PlaylistUpdate.cxx @@ -35,7 +35,7 @@ UpdatePlaylistSong(const Database &db, DetachedSong &song) const LightSong *original; try { - original = db.GetSong(song.GetURI(), IgnoreError()); + original = db.GetSong(song.GetURI()); } catch (const std::runtime_error &e) { return false; } diff --git a/src/sticker/SongSticker.cxx b/src/sticker/SongSticker.cxx index 121494af9..f144327cf 100644 --- a/src/sticker/SongSticker.cxx +++ b/src/sticker/SongSticker.cxx @@ -93,11 +93,9 @@ sticker_song_find_cb(const char *uri, const char *value, void *user_data) const Database *db = data->db; try { - const LightSong *song = db->GetSong(uri, IgnoreError()); - if (song != nullptr) { - data->func(*song, value, data->user_data); - db->ReturnSong(song); - } + const LightSong *song = db->GetSong(uri); + data->func(*song, value, data->user_data); + db->ReturnSong(song); } catch (const std::runtime_error &e) { } } diff --git a/test/test_translate_song.cxx b/test/test_translate_song.cxx index b6be581e4..e7de78fb2 100644 --- a/test/test_translate_song.cxx +++ b/test/test_translate_song.cxx @@ -111,8 +111,7 @@ static const char *uri2 = "foo/bar.ogg"; DetachedSong * DatabaseDetachSong(gcc_unused const Database &db, gcc_unused const Storage &_storage, - const char *uri, - gcc_unused Error &error) + const char *uri) { if (strcmp(uri, uri2) == 0) return new DetachedSong(uri, MakeTag2a());