From c13810ebaa7075284691eb2add089ba407dfd1ea Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 7 Feb 2014 00:29:07 +0100 Subject: [PATCH] Mapper: move map_song_detach() to db/DatabaseSong.cxx Use Storage::MapUTF8() internally, don't use global variables. --- src/DetachedSong.hxx | 3 +++ src/Mapper.cxx | 15 --------------- src/Mapper.hxx | 8 -------- src/PlaylistSave.cxx | 2 +- src/SongLoader.cxx | 6 ++++-- src/SongLoader.hxx | 13 ++++++++----- src/StateFile.cxx | 5 +++-- src/client/Client.cxx | 6 ++++++ src/client/Client.hxx | 5 +++++ src/command/DatabaseCommands.cxx | 3 ++- src/command/PlaylistCommands.cxx | 3 ++- src/db/DatabasePlaylist.cxx | 13 ++++++++----- src/db/DatabasePlaylist.hxx | 5 +++-- src/db/DatabaseQueue.cxx | 7 ++++--- src/db/DatabaseSong.cxx | 23 ++++++++++++++++++++--- src/db/DatabaseSong.hxx | 13 ++++++++++++- test/test_translate_song.cxx | 22 ++++++++++++++++------ 17 files changed, 97 insertions(+), 55 deletions(-) diff --git a/src/DetachedSong.hxx b/src/DetachedSong.hxx index c01f32ea5..7ea0bc8d8 100644 --- a/src/DetachedSong.hxx +++ b/src/DetachedSong.hxx @@ -30,9 +30,12 @@ #include struct LightSong; +class Storage; class DetachedSong { friend DetachedSong map_song_detach(const LightSong &song); + friend DetachedSong DatabaseDetachSong(const Storage &db, + const LightSong &song); /** * An UTF-8-encoded URI referring to the song file. This can diff --git a/src/Mapper.cxx b/src/Mapper.cxx index 1178313d9..4e7efff67 100644 --- a/src/Mapper.cxx +++ b/src/Mapper.cxx @@ -234,21 +234,6 @@ map_directory_child_fs(const Directory &directory, const char *name) return AllocatedPath::Build(parent_fs, name_fs); } -DetachedSong -map_song_detach(const LightSong &song) -{ - DetachedSong detached(song); - assert(detached.IsInDatabase()); - - if (!detached.HasRealURI()) { - const auto uri = song.GetURI(); - detached.SetRealURI(PathTraitsUTF8::Build(music_dir_utf8.c_str(), - uri.c_str())); - } - - return detached; -} - AllocatedPath map_song_fs(const Song &song) { diff --git a/src/Mapper.hxx b/src/Mapper.hxx index a94a4fbb5..3ddee5568 100644 --- a/src/Mapper.hxx +++ b/src/Mapper.hxx @@ -91,14 +91,6 @@ gcc_pure AllocatedPath map_uri_fs(const char *uri); -/** - * "Detach" the #Song object, i.e. convert it to a #DetachedSong - * instance. - */ -gcc_pure -DetachedSong -map_song_detach(const LightSong &song); - /** * Determines the file system path of a song. This must not be a * remote song. diff --git a/src/PlaylistSave.cxx b/src/PlaylistSave.cxx index d3369c9b6..1e5094b18 100644 --- a/src/PlaylistSave.cxx +++ b/src/PlaylistSave.cxx @@ -118,7 +118,7 @@ playlist_load_spl(struct playlist &playlist, PlayerControl &pc, if (end_index > contents.size()) end_index = contents.size(); - const SongLoader loader(nullptr); + const SongLoader loader(nullptr, nullptr); for (unsigned i = start_index; i < end_index; ++i) { const auto &uri_utf8 = contents[i]; diff --git a/src/SongLoader.cxx b/src/SongLoader.cxx index 08aa01296..f2cf216a7 100644 --- a/src/SongLoader.cxx +++ b/src/SongLoader.cxx @@ -36,7 +36,8 @@ #ifdef ENABLE_DATABASE SongLoader::SongLoader(const Client &_client) - :client(&_client), db(_client.GetDatabase(IgnoreError())) {} + :client(&_client), db(_client.GetDatabase(IgnoreError())), + storage(_client.GetStorage()) {} #endif @@ -100,7 +101,8 @@ SongLoader::LoadSong(const char *uri_utf8, Error &error) const #ifdef ENABLE_DATABASE if (db != nullptr) - return DatabaseDetachSong(*db, uri_utf8, error); + return DatabaseDetachSong(*db, *storage, + uri_utf8, error); #endif error.Set(playlist_domain, int(PlaylistResult::NO_SUCH_SONG), diff --git a/src/SongLoader.hxx b/src/SongLoader.hxx index 9914db8e6..c8cd87024 100644 --- a/src/SongLoader.hxx +++ b/src/SongLoader.hxx @@ -27,6 +27,7 @@ class Client; class Database; +class Storage; class DetachedSong; class Error; @@ -41,19 +42,21 @@ class SongLoader { #ifdef ENABLE_DATABASE const Database *const db; + const Storage *const storage; #endif public: #ifdef ENABLE_DATABASE explicit SongLoader(const Client &_client); - explicit SongLoader(const Database *_db) - :client(nullptr), db(_db) {} - explicit SongLoader(const Client &_client, const Database *_db) - :client(&_client), db(_db) {} + SongLoader(const Database *_db, const Storage *_storage) + :client(nullptr), db(_db), storage(_storage) {} + SongLoader(const Client &_client, const Database *_db, + const Storage *_storage) + :client(&_client), db(_db), storage(_storage) {} #else explicit SongLoader(const Client &_client) :client(&_client) {} - explicit SongLoader(std::nullptr_t) + explicit SongLoader(std::nullptr_t, std::nullptr_t) :client(nullptr) {} #endif diff --git a/src/StateFile.cxx b/src/StateFile.cxx index e46af1c3e..a1d8945c4 100644 --- a/src/StateFile.cxx +++ b/src/StateFile.cxx @@ -99,9 +99,10 @@ StateFile::Read() } #ifdef ENABLE_DATABASE - const SongLoader song_loader(partition.instance.database); + const SongLoader song_loader(partition.instance.database, + partition.instance.storage); #else - const SongLoader song_loader(nullptr); + const SongLoader song_loader(nullptr, nullptr); #endif const char *line; diff --git a/src/client/Client.cxx b/src/client/Client.cxx index c62c759e9..01ead4645 100644 --- a/src/client/Client.cxx +++ b/src/client/Client.cxx @@ -33,4 +33,10 @@ Client::GetDatabase(Error &error) const return partition.instance.GetDatabase(error); } +const Storage * +Client::GetStorage() const +{ + return partition.instance.storage; +} + #endif diff --git a/src/client/Client.hxx b/src/client/Client.hxx index 006ffc98c..56cd947ff 100644 --- a/src/client/Client.hxx +++ b/src/client/Client.hxx @@ -39,6 +39,7 @@ class EventLoop; class Path; struct Partition; class Database; +class Storage; class Client final : private FullyBufferedSocket, TimeoutMonitor { public: @@ -173,8 +174,12 @@ public: /** * Wrapper for Instance::GetDatabase(). */ + gcc_pure const Database *GetDatabase(Error &error) const; + gcc_pure + const Storage *GetStorage() const; + private: /* virtual methods from class BufferedSocket */ virtual InputResult OnSocketInput(void *data, size_t length) override; diff --git a/src/command/DatabaseCommands.cxx b/src/command/DatabaseCommands.cxx index ef6fb922e..89fe2e050 100644 --- a/src/command/DatabaseCommands.cxx +++ b/src/command/DatabaseCommands.cxx @@ -124,7 +124,8 @@ handle_searchaddpl(Client &client, int argc, char *argv[]) if (db == nullptr) return print_error(client, error); - return search_add_to_playlist(*db, "", playlist, &filter, error) + return search_add_to_playlist(*db, *client.GetStorage(), + "", playlist, &filter, error) ? CommandResult::OK : print_error(client, error); } diff --git a/src/command/PlaylistCommands.cxx b/src/command/PlaylistCommands.cxx index 0997cc68d..6406b0770 100644 --- a/src/command/PlaylistCommands.cxx +++ b/src/command/PlaylistCommands.cxx @@ -196,7 +196,8 @@ handle_playlistadd(Client &client, gcc_unused int argc, char *argv[]) if (db == nullptr) return print_error(client, error); - success = search_add_to_playlist(*db, uri, playlist, nullptr, + success = search_add_to_playlist(*db, *client.GetStorage(), + uri, playlist, nullptr, error); #else success = false; diff --git a/src/db/DatabasePlaylist.cxx b/src/db/DatabasePlaylist.cxx index 814901227..90a7f7b1a 100644 --- a/src/db/DatabasePlaylist.cxx +++ b/src/db/DatabasePlaylist.cxx @@ -19,24 +19,26 @@ #include "config.h" #include "DatabasePlaylist.hxx" +#include "DatabaseSong.hxx" #include "Selection.hxx" #include "PlaylistFile.hxx" #include "DatabasePlugin.hxx" #include "DetachedSong.hxx" -#include "Mapper.hxx" +#include "storage/StorageInterface.hxx" #include static bool -AddSong(const char *playlist_path_utf8, +AddSong(const Storage &storage, const char *playlist_path_utf8, const LightSong &song, Error &error) { - return spl_append_song(playlist_path_utf8, map_song_detach(song), + return spl_append_song(playlist_path_utf8, + DatabaseDetachSong(storage, song), error); } bool -search_add_to_playlist(const Database &db, +search_add_to_playlist(const Database &db, const Storage &storage, const char *uri, const char *playlist_path_utf8, const SongFilter *filter, Error &error) @@ -44,6 +46,7 @@ search_add_to_playlist(const Database &db, const DatabaseSelection selection(uri, true, filter); using namespace std::placeholders; - const auto f = std::bind(AddSong, playlist_path_utf8, _1, _2); + const auto f = std::bind(AddSong, std::ref(storage), + playlist_path_utf8, _1, _2); return db.Visit(selection, f, error); } diff --git a/src/db/DatabasePlaylist.hxx b/src/db/DatabasePlaylist.hxx index 5feafa190..9dc3526bb 100644 --- a/src/db/DatabasePlaylist.hxx +++ b/src/db/DatabasePlaylist.hxx @@ -23,12 +23,13 @@ #include "Compiler.h" class Database; +class Storage; class SongFilter; class Error; -gcc_nonnull(2,3) +gcc_nonnull(3,4) bool -search_add_to_playlist(const Database &db, +search_add_to_playlist(const Database &db, const Storage &storage, const char *uri, const char *path_utf8, const SongFilter *filter, Error &error); diff --git a/src/db/DatabaseQueue.cxx b/src/db/DatabaseQueue.cxx index f2a0951a6..77fd57fe3 100644 --- a/src/db/DatabaseQueue.cxx +++ b/src/db/DatabaseQueue.cxx @@ -19,22 +19,23 @@ #include "config.h" #include "DatabaseQueue.hxx" -#include "DatabaseGlue.hxx" +#include "DatabaseSong.hxx" #include "DatabasePlugin.hxx" #include "Partition.hxx" #include "Instance.hxx" #include "util/Error.hxx" #include "DetachedSong.hxx" -#include "Mapper.hxx" #include static bool AddToQueue(Partition &partition, const LightSong &song, Error &error) { + const Storage &storage = *partition.instance.storage; PlaylistResult result = partition.playlist.AppendSong(partition.pc, - map_song_detach(song), + DatabaseDetachSong(storage, + song), nullptr); if (result != PlaylistResult::SUCCESS) { error.Set(playlist_domain, int(result), "Playlist error"); diff --git a/src/db/DatabaseSong.cxx b/src/db/DatabaseSong.cxx index f6229194b..d9adad7a0 100644 --- a/src/db/DatabaseSong.cxx +++ b/src/db/DatabaseSong.cxx @@ -19,18 +19,35 @@ #include "config.h" #include "DatabaseSong.hxx" +#include "LightSong.hxx" #include "DatabasePlugin.hxx" #include "DetachedSong.hxx" -#include "Mapper.hxx" +#include "storage/StorageInterface.hxx" + +DetachedSong +DatabaseDetachSong(const Storage &storage, const LightSong &song) +{ + DetachedSong detached(song); + assert(detached.IsInDatabase()); + + if (!detached.HasRealURI()) { + const auto uri = song.GetURI(); + detached.SetRealURI(storage.MapUTF8(uri.c_str())); + } + + return detached; +} DetachedSong * -DatabaseDetachSong(const Database &db, const char *uri, Error &error) +DatabaseDetachSong(const Database &db, const Storage &storage, const char *uri, + Error &error) { const LightSong *tmp = db.GetSong(uri, error); if (tmp == nullptr) return nullptr; - DetachedSong *song = new DetachedSong(map_song_detach(*tmp)); + DetachedSong *song = new DetachedSong(DatabaseDetachSong(storage, + *tmp)); db.ReturnSong(tmp); return song; } diff --git a/src/db/DatabaseSong.hxx b/src/db/DatabaseSong.hxx index 1197068bc..4daaf4047 100644 --- a/src/db/DatabaseSong.hxx +++ b/src/db/DatabaseSong.hxx @@ -22,10 +22,20 @@ #include "Compiler.h" +struct LightSong; class Database; +class Storage; class DetachedSong; class Error; +/** + * "Detach" the #Song object, i.e. convert it to a #DetachedSong + * instance. + */ +gcc_pure +DetachedSong +DatabaseDetachSong(const Storage &storage, const LightSong &song); + /** * Look up a song in the database and convert it to a #DetachedSong * instance. The caller is responsible for freeing it. @@ -34,6 +44,7 @@ class Error; */ gcc_malloc gcc_nonnull_all DetachedSong * -DatabaseDetachSong(const Database &db, const char *uri, Error &error); +DatabaseDetachSong(const Database &db, const Storage &storage, const char *uri, + Error &error); #endif diff --git a/test/test_translate_song.cxx b/test/test_translate_song.cxx index bc1ae82d6..aacc25820 100644 --- a/test/test_translate_song.cxx +++ b/test/test_translate_song.cxx @@ -117,7 +117,9 @@ static const char *uri1 = "/foo/bar.ogg"; static const char *uri2 = "foo/bar.ogg"; DetachedSong * -DatabaseDetachSong(gcc_unused const Database &db, const char *uri, +DatabaseDetachSong(gcc_unused const Database &db, + gcc_unused const Storage &storage, + const char *uri, gcc_unused Error &error) { if (strcmp(uri, uri2) == 0) @@ -143,6 +145,12 @@ Client::GetDatabase(gcc_unused Error &error) const return reinterpret_cast(this); } +const Storage * +Client::GetStorage() const +{ + return reinterpret_cast(this); +} + bool Client::AllowFile(gcc_unused Path path_fs, gcc_unused Error &error) const { @@ -217,7 +225,7 @@ class TranslateSongTest : public CppUnit::TestFixture { void TestAbsoluteURI() { DetachedSong song1("http://example.com/foo.ogg"); auto se = ToString(song1); - const SongLoader loader(nullptr); + const SongLoader loader(nullptr, nullptr); CPPUNIT_ASSERT(playlist_check_translate_song(song1, "/ignored", loader)); CPPUNIT_ASSERT_EQUAL(se, ToString(song1)); @@ -236,14 +244,15 @@ class TranslateSongTest : public CppUnit::TestFixture { auto s1 = ToString(song1); auto se = ToString(DetachedSong(uri1, MakeTag1c())); - const SongLoader loader(nullptr); + const SongLoader loader(nullptr, nullptr); CPPUNIT_ASSERT(playlist_check_translate_song(song1, "/ignored", loader)); CPPUNIT_ASSERT_EQUAL(se, ToString(song1)); } void TestInDatabase() { - const SongLoader loader(reinterpret_cast(1)); + const SongLoader loader(reinterpret_cast(1), + reinterpret_cast(2)); DetachedSong song1("doesntexist"); CPPUNIT_ASSERT(!playlist_check_translate_song(song1, nullptr, @@ -266,9 +275,10 @@ class TranslateSongTest : public CppUnit::TestFixture { void TestRelative() { const Database &db = *reinterpret_cast(1); - const SongLoader secure_loader(&db); + const Storage &storage = *reinterpret_cast(2); + const SongLoader secure_loader(&db, &storage); const SongLoader insecure_loader(*reinterpret_cast(1), - &db); + &db, &storage); /* map to music_directory */ DetachedSong song1("bar.ogg", MakeTag2b());