Commit 3fe1e17a authored by Hugo Beauzée-Luyssen's avatar Hugo Beauzée-Luyssen

Remove the concept of Cache.

Reading from the DB is "cheap", and the cache hurts the maintainability
of the code.
This removes the CachePolicy and CacheColumn field from TablePolicy.
Instead, a PrimaryKeyColumn has been added to TablePolicy
parent 7f3c02e2
......@@ -103,7 +103,6 @@ class IMediaLibrary
virtual bool deleteFolder( FolderPtr folder ) = 0;
virtual LabelPtr createLabel( const std::string& label ) = 0;
virtual bool deleteLabel( const std::string& label ) = 0;
virtual bool deleteLabel( LabelPtr label ) = 0;
virtual std::vector<MediaPtr> audioFiles() = 0;
virtual std::vector<MediaPtr> videoFiles() = 0;
......
......@@ -30,7 +30,7 @@
#include "database/SqliteTools.h"
const std::string policy::AlbumTable::Name = "Album";
const std::string policy::AlbumTable::CacheColumn = "id_album";
const std::string policy::AlbumTable::PrimaryKeyColumn = "id_album";
unsigned int Album::* const policy::AlbumTable::PrimaryKey = &Album::m_id;
Album::Album(DBConnection dbConnection, sqlite::Row& row)
......@@ -254,9 +254,9 @@ bool Album::createTable(DBConnection dbConnection )
"id_artist INTEGER,"
"PRIMARY KEY (id_album, id_artist),"
"FOREIGN KEY(id_album) REFERENCES " + policy::AlbumTable::Name + "("
+ policy::AlbumTable::CacheColumn + ") ON DELETE CASCADE,"
+ policy::AlbumTable::PrimaryKeyColumn + ") ON DELETE CASCADE,"
"FOREIGN KEY(id_artist) REFERENCES " + policy::ArtistTable::Name + "("
+ policy::ArtistTable::CacheColumn + ") ON DELETE CASCADE"
+ policy::ArtistTable::PrimaryKeyColumn + ") ON DELETE CASCADE"
")";
return sqlite::Tools::executeRequest( dbConnection, req ) &&
sqlite::Tools::executeRequest( dbConnection, reqRel );
......
......@@ -42,7 +42,7 @@ namespace policy
struct AlbumTable
{
static const std::string Name;
static const std::string CacheColumn;
static const std::string PrimaryKeyColumn;
static unsigned int Album::*const PrimaryKey;
};
}
......
......@@ -27,7 +27,7 @@
#include "logging/Logger.h"
const std::string policy::AlbumTrackTable::Name = "AlbumTrack";
const std::string policy::AlbumTrackTable::CacheColumn = "id_track";
const std::string policy::AlbumTrackTable::PrimaryKeyColumn = "id_track";
unsigned int AlbumTrack::* const policy::AlbumTrackTable::PrimaryKey = &AlbumTrack::m_id;
AlbumTrack::AlbumTrack(DBConnection dbConnection, sqlite::Row& row )
......
......@@ -39,7 +39,7 @@ namespace policy
struct AlbumTrackTable
{
static const std::string Name;
static const std::string CacheColumn;
static const std::string PrimaryKeyColumn;
static unsigned int AlbumTrack::*const PrimaryKey;
};
}
......
......@@ -28,7 +28,7 @@
#include "database/SqliteTools.h"
const std::string policy::ArtistTable::Name = "artist";
const std::string policy::ArtistTable::CacheColumn = "id_artist";
const std::string policy::ArtistTable::PrimaryKeyColumn = "id_artist";
unsigned int Artist::*const policy::ArtistTable::PrimaryKey = &Artist::m_id;
......@@ -145,7 +145,7 @@ std::shared_ptr<Album> Artist::unknownAlbum()
{
static const std::string req = "SELECT * FROM " + policy::AlbumTable::Name +
" WHERE artist_id = ? AND title IS NULL";
auto album = Album::fetchOne( m_dbConnection, req, m_id );
auto album = Album::fetch( m_dbConnection, req, m_id );
if ( album == nullptr )
{
album = Album::createUnknownAlbum( m_dbConnection, this );
......@@ -153,7 +153,7 @@ std::shared_ptr<Album> Artist::unknownAlbum()
return nullptr;
if ( updateNbAlbum( 1 ) == false )
{
Album::destroy( m_dbConnection, album.get() );
Album::destroy( m_dbConnection, album->id() );
return nullptr;
}
}
......@@ -178,7 +178,7 @@ bool Artist::createTable( DBConnection dbConnection )
"FOREIGN KEY(id_media) REFERENCES " + policy::MediaTable::Name +
"(id_media) ON DELETE CASCADE,"
"FOREIGN KEY(id_artist) REFERENCES " + policy::ArtistTable::Name + "("
+ policy::ArtistTable::CacheColumn + ") ON DELETE CASCADE"
+ policy::ArtistTable::PrimaryKeyColumn + ") ON DELETE CASCADE"
")";
return sqlite::Tools::executeRequest( dbConnection, req ) &&
sqlite::Tools::executeRequest( dbConnection, reqRel );
......
......@@ -35,7 +35,7 @@ namespace policy
struct ArtistTable
{
static const std::string Name;
static const std::string CacheColumn;
static const std::string PrimaryKeyColumn;
static unsigned int Artist::*const PrimaryKey;
};
}
......
......@@ -29,19 +29,8 @@
namespace policy
{
const std::string FolderTable::Name = "Folder";
const std::string FolderTable::CacheColumn = "path";
const std::string FolderTable::PrimaryKeyColumn = "id_folder";
unsigned int Folder::* const FolderTable::PrimaryKey = &Folder::m_id;
const FolderCache::KeyType&FolderCache::key( const IFolder* self )
{
return self->path();
}
FolderCache::KeyType FolderCache::key(const sqlite::Row& row )
{
return row.load<FolderCache::KeyType>( 1 );
}
}
Folder::Folder(DBConnection dbConnection, sqlite::Row& row )
......@@ -90,6 +79,12 @@ std::shared_ptr<Folder> Folder::create( DBConnection connection, const std::stri
return self;
}
std::shared_ptr<Folder> Folder::fromPath( DBConnection conn, const std::string& path )
{
const std::string req = "SELECT * FROM " + policy::FolderTable::Name + " WHERE path = ?";
return fetch( conn, req, path );
}
unsigned int Folder::id() const
{
return m_id;
......@@ -116,10 +111,7 @@ std::vector<FolderPtr> Folder::folders()
FolderPtr Folder::parent()
{
//FIXME: use path to be able to fetch from cache?
static const std::string req = "SELECT * FROM " + policy::FolderTable::Name +
" WHERE id_folder = ?";
return fetchOne( m_dbConection, req, m_parent );
return fetch( m_dbConection, m_parent );
}
unsigned int Folder::lastModificationDate()
......
......@@ -39,22 +39,15 @@ namespace policy
struct FolderTable
{
static const std::string Name;
static const std::string CacheColumn;
static const std::string PrimaryKeyColumn;
static unsigned int Folder::*const PrimaryKey;
};
struct FolderCache
{
using KeyType = std::string;
static const KeyType& key( const IFolder* self );
static KeyType key( const sqlite::Row& row );
};
}
class Folder : public IFolder, public Table<Folder, policy::FolderTable, policy::FolderCache>
class Folder : public IFolder, public Table<Folder, policy::FolderTable>
{
using _Cache = Table<Folder, policy::FolderTable, policy::FolderCache>;
using _Cache = Table<Folder, policy::FolderTable>;
public:
Folder( DBConnection dbConnection, sqlite::Row& row );
......@@ -63,6 +56,8 @@ public:
static bool createTable( DBConnection connection );
static std::shared_ptr<Folder> create( DBConnection connection, const std::string& path, time_t lastModificationDate, bool isRemovable, unsigned int parentId );
static std::shared_ptr<Folder> fromPath( DBConnection conn, const std::string& path );
virtual unsigned int id() const override;
virtual const std::string& path() const override;
virtual std::vector<MediaPtr> files() override;
......
......@@ -29,7 +29,7 @@
#include "database/SqliteTools.h"
const std::string policy::LabelTable::Name = "Label";
const std::string policy::LabelTable::CacheColumn = "name";
const std::string policy::LabelTable::PrimaryKeyColumn = "id_label";
unsigned int Label::* const policy::LabelTable::PrimaryKey = &Label::m_id;
Label::Label( DBConnection dbConnection, sqlite::Row& row )
......
......@@ -37,7 +37,7 @@ namespace policy
struct LabelTable
{
static const std::string Name;
static const std::string CacheColumn;
static const std::string PrimaryKeyColumn;
static unsigned int Label::*const PrimaryKey;
};
......@@ -50,9 +50,9 @@ struct LabelCachePolicy
}
class Label : public ILabel, public Table<Label, policy::LabelTable, policy::LabelCachePolicy>
class Label : public ILabel, public Table<Label, policy::LabelTable>
{
using _Cache = Table<Label, policy::LabelTable, policy::LabelCachePolicy>;
using _Cache = Table<Label, policy::LabelTable>;
public:
Label( DBConnection dbConnection, sqlite::Row& row );
......
......@@ -40,7 +40,7 @@
#include "filesystem/IFile.h"
const std::string policy::MediaTable::Name = "Media";
const std::string policy::MediaTable::CacheColumn = "mrl";
const std::string policy::MediaTable::PrimaryKeyColumn = "id_media";
unsigned int Media::* const policy::MediaTable::PrimaryKey = &Media::m_id;
Media::Media( DBConnection dbConnection, sqlite::Row& row )
......@@ -95,7 +95,7 @@ AlbumTrackPtr Media::albumTrack()
{
std::string req = "SELECT * FROM " + policy::AlbumTrackTable::Name +
" WHERE media_id = ?";
m_albumTrack = AlbumTrack::fetchOne( m_dbConnection, req, m_id );
m_albumTrack = AlbumTrack::fetch( m_dbConnection, req, m_id );
}
return m_albumTrack;
}
......@@ -347,13 +347,3 @@ bool Media::removeLabel( LabelPtr label )
const char* req = "DELETE FROM LabelFileRelation WHERE id_label = ? AND id_media = ?";
return sqlite::Tools::executeDelete( m_dbConnection, req, label->id(), m_id );
}
const std::string& policy::MediaCache::key( const IMedia* self )
{
return self->mrl();
}
std::string policy::MediaCache::key( const sqlite::Row& row )
{
return row.load<std::string>( 5 );
}
......@@ -46,21 +46,15 @@ namespace policy
struct MediaTable
{
static const std::string Name;
static const std::string CacheColumn;
static const std::string PrimaryKeyColumn;
static unsigned int Media::*const PrimaryKey;
};
struct MediaCache
{
typedef std::string KeyType;
static const std::string& key(const IMedia* self);
static std::string key( const sqlite::Row& row );
};
}
class Media : public IMedia, public Table<Media, policy::MediaTable, policy::MediaCache>
class Media : public IMedia, public Table<Media, policy::MediaTable>
{
private:
using _Cache = Table<Media, policy::MediaTable, policy::MediaCache>;
using _Cache = Table<Media, policy::MediaTable>;
public:
// Those should be private, however the standard states that the expression
......
......@@ -190,7 +190,9 @@ std::vector<MediaPtr> MediaLibrary::videoFiles()
MediaPtr MediaLibrary::file( const std::string& path )
{
return Media::fetch( m_dbConnection.get(), path );
static const std::string req = "SELECT * FROM " + policy::MediaTable::Name +
" WHERE mrl = ?";
return Media::fetch( m_dbConnection.get(), req, path );
}
std::shared_ptr<Media> MediaLibrary::addFile( const std::string& path, FolderPtr parentFolder )
......@@ -240,17 +242,17 @@ std::shared_ptr<Media> MediaLibrary::addFile( const std::string& path, FolderPtr
FolderPtr MediaLibrary::folder( const std::string& path )
{
return Folder::fetch( m_dbConnection.get(), path );
return Folder::fromPath( m_dbConnection.get(), path );
}
bool MediaLibrary::deleteFile( const Media* file )
{
return Media::destroy( m_dbConnection.get(), file );
return Media::destroy( m_dbConnection.get(), file->id() );
}
bool MediaLibrary::deleteFolder( FolderPtr folder )
{
if ( Folder::destroy( m_dbConnection.get(), folder.get() ) == false )
if ( Folder::destroy( m_dbConnection.get(), folder->id() ) == false )
return false;
Media::clear();
return true;
......@@ -261,14 +263,9 @@ LabelPtr MediaLibrary::createLabel( const std::string& label )
return Label::create( m_dbConnection.get(), label );
}
bool MediaLibrary::deleteLabel( const std::string& text )
{
return Label::destroy( m_dbConnection.get(), text );
}
bool MediaLibrary::deleteLabel( LabelPtr label )
{
return Label::destroy( m_dbConnection.get(), label.get() );
return Label::destroy( m_dbConnection.get(), label->id() );
}
AlbumPtr MediaLibrary::album( unsigned int id )
......@@ -292,7 +289,7 @@ ShowPtr MediaLibrary::show(const std::string& name)
{
static const std::string req = "SELECT * FROM " + policy::ShowTable::Name
+ " WHERE name = ?";
return Show::fetchOne( m_dbConnection.get(), req, name );
return Show::fetch( m_dbConnection.get(), req, name );
}
std::shared_ptr<Show> MediaLibrary::createShow(const std::string& name)
......@@ -304,7 +301,7 @@ MoviePtr MediaLibrary::movie( const std::string& title )
{
static const std::string req = "SELECT * FROM " + policy::MovieTable::Name
+ " WHERE title = ?";
return Movie::fetchOne( m_dbConnection.get(), req, title );
return Movie::fetch( m_dbConnection.get(), req, title );
}
std::shared_ptr<Movie> MediaLibrary::createMovie( const std::string& title )
......@@ -321,7 +318,7 @@ ArtistPtr MediaLibrary::artist( const std::string& name )
{
static const std::string req = "SELECT * FROM " + policy::ArtistTable::Name
+ " WHERE name = ?";
return Artist::fetchOne( m_dbConnection.get(), req, name );
return Artist::fetch( m_dbConnection.get(), req, name );
}
std::shared_ptr<Artist> MediaLibrary::createArtist( const std::string& name )
......
......@@ -58,7 +58,6 @@ class MediaLibrary : public IMediaLibrary
virtual bool deleteFolder( FolderPtr folder ) override;
virtual LabelPtr createLabel( const std::string& label ) override;
virtual bool deleteLabel(const std::string& text ) override;
virtual bool deleteLabel( LabelPtr label ) override;
virtual AlbumPtr album( unsigned int id ) override;
......
......@@ -25,7 +25,7 @@
#include "database/SqliteTools.h"
const std::string policy::MovieTable::Name = "Movie";
const std::string policy::MovieTable::CacheColumn = "id_movie";
const std::string policy::MovieTable::PrimaryKeyColumn = "id_movie";
unsigned int Movie::* const policy::MovieTable::PrimaryKey = &Movie::m_id;
Movie::Movie(DBConnection dbConnection, sqlite::Row& row )
......
......@@ -34,7 +34,7 @@ namespace policy
struct MovieTable
{
static const std::string Name;
static const std::string CacheColumn;
static const std::string PrimaryKeyColumn;
static unsigned int Movie::*const PrimaryKey;
};
}
......
......@@ -25,7 +25,7 @@
#include "database/SqliteTools.h"
const std::string policy::ShowTable::Name = "Show";
const std::string policy::ShowTable::CacheColumn = "id_show";
const std::string policy::ShowTable::PrimaryKeyColumn = "id_show";
unsigned int Show::* const policy::ShowTable::PrimaryKey = &Show::m_id;
Show::Show( DBConnection dbConnection, sqlite::Row& row )
......
......@@ -37,7 +37,7 @@ namespace policy
struct ShowTable
{
static const std::string Name;
static const std::string CacheColumn;
static const std::string PrimaryKeyColumn;
static unsigned int Show::*const PrimaryKey;
};
}
......
......@@ -26,7 +26,7 @@
#include "Media.h"
const std::string policy::ShowEpisodeTable::Name = "ShowEpisode";
const std::string policy::ShowEpisodeTable::CacheColumn = "show_id";
const std::string policy::ShowEpisodeTable::PrimaryKeyColumn = "show_id";
unsigned int ShowEpisode::* const policy::ShowEpisodeTable::PrimaryKey = &ShowEpisode::m_id;
ShowEpisode::ShowEpisode(DBConnection dbConnection, sqlite::Row& row )
......
......@@ -38,7 +38,7 @@ namespace policy
struct ShowEpisodeTable
{
static const std::string Name;
static const std::string CacheColumn;
static const std::string PrimaryKeyColumn;
static unsigned int ShowEpisode::*const PrimaryKey;
};
}
......
......@@ -30,23 +30,7 @@
#include "SqliteTools.h"
class PrimaryKeyCacheKeyPolicy
{
public:
typedef unsigned int KeyType;
template <typename T>
static unsigned int key( const T* self )
{
return self->id();
}
static unsigned int key( const sqlite::Row& row )
{
return row.load<unsigned int>( 0 );
}
};
template <typename IMPL, typename CACHEPOLICY>
template <typename IMPL>
class Cache
{
using lock_t = std::unique_lock<std::recursive_mutex>;
......@@ -57,7 +41,7 @@ public:
return lock_t{ Mutex };
}
static std::shared_ptr<IMPL> load( const typename CACHEPOLICY::KeyType& key )
static std::shared_ptr<IMPL> load( unsigned int key )
{
auto it = Store.find( key );
if ( it == Store.end() )
......@@ -65,23 +49,15 @@ public:
return it->second;
}
template <typename T>
static std::shared_ptr<IMPL> load( const T& value )
{
auto key = CACHEPOLICY::key( value );
return load( key );
}
static void store( const std::shared_ptr<IMPL> value )
{
auto key = CACHEPOLICY::key( value.get() );
Store[key] = value;
Store[value->id()] = value;
}
/**
* @brief discard Discard a record from the cache
*/
static bool discard( const typename CACHEPOLICY::KeyType& key )
static bool discard( unsigned int key )
{
auto it = Store.find( key );
if ( it == Store.end() )
......@@ -96,16 +72,16 @@ public:
}
private:
static std::unordered_map<typename CACHEPOLICY::KeyType, std::shared_ptr<IMPL>> Store;
static std::unordered_map<unsigned int, std::shared_ptr<IMPL>> Store;
static std::recursive_mutex Mutex;
};
template <typename IMPL, typename CACHEPOLICY>
std::unordered_map<typename CACHEPOLICY::KeyType, std::shared_ptr<IMPL>>
Cache<IMPL, CACHEPOLICY>::Store;
template <typename IMPL>
std::unordered_map<unsigned int, std::shared_ptr<IMPL>>
Cache<IMPL>::Store;
template <typename IMPL, typename CACHEPOLICY>
std::recursive_mutex Cache<IMPL, CACHEPOLICY>::Mutex;
template <typename IMPL>
std::recursive_mutex Cache<IMPL>::Mutex;
#endif // CACHE_H
......@@ -24,32 +24,23 @@
#include "Cache.h"
template <typename IMPL, typename TABLEPOLICY, typename CACHEPOLICY = PrimaryKeyCacheKeyPolicy >
template <typename IMPL, typename TABLEPOLICY>
class Table
{
using _Cache = Cache<IMPL, CACHEPOLICY>;
using _Cache = Cache<IMPL>;
public:
static std::shared_ptr<IMPL> fetch( DBConnection dbConnection, const typename CACHEPOLICY::KeyType& key )
template <typename... Args>
static std::shared_ptr<IMPL> fetch( DBConnection dbConnection, const std::string& req, Args&&... args )
{
auto l = _Cache::lock();
auto res = _Cache::load( key );
if ( res != nullptr )
return res;
static const std::string req = "SELECT * FROM " + TABLEPOLICY::Name +
" WHERE " + TABLEPOLICY::CacheColumn + " = ?";
res = sqlite::Tools::fetchOne<IMPL>( dbConnection, req, key );
if ( res == nullptr )
return nullptr;
_Cache::store( res );
return res;
return sqlite::Tools::fetchOne<IMPL>( dbConnection, req, std::forward<Args>( args )... );
}
template <typename... Args>
static std::shared_ptr<IMPL> fetchOne( DBConnection dbConnection, const std::string& req, Args&&... args )
static std::shared_ptr<IMPL> fetch( DBConnection dbConnection, unsigned int pkValue )
{
return sqlite::Tools::fetchOne<IMPL>( dbConnection, req, std::forward<Args>( args )... );
static std::string req = "SELECT * FROM " + TABLEPOLICY::Name + " WHERE " +
TABLEPOLICY::PrimaryKeyColumn + " = ?";
return sqlite::Tools::fetchOne<IMPL>( dbConnection, req, pkValue );
}
/*
......@@ -76,7 +67,8 @@ class Table
{
auto l = _Cache::lock();
auto res = _Cache::load( row );
auto key = row.load<unsigned int>( 0 );
auto res = _Cache::load( key );
if ( res != nullptr )
return res;
res = std::make_shared<IMPL>( dbConnection, row );
......@@ -84,20 +76,22 @@ class Table
return res;
}
static bool destroy( DBConnection dbConnection, const typename CACHEPOLICY::KeyType& key )
template <typename... Args>
static bool destroy( DBConnection dbConnection, unsigned int pkValue )
{
auto l = _Cache::lock();
_Cache::discard( key );
static const std::string req = "DELETE FROM " + TABLEPOLICY::Name + " WHERE " +
TABLEPOLICY::CacheColumn + " = ?";
return sqlite::Tools::executeDelete( dbConnection, req, key );
}
template <typename T>
static bool destroy( DBConnection dbConnection, const T* self )
{
const auto& key = CACHEPOLICY::key( self );
return destroy( dbConnection, key );
static const std::string req = "DELETE FROM " + TABLEPOLICY::Name + " WHERE "
+ TABLEPOLICY::PrimaryKeyColumn + " = ?";
auto res = sqlite::Tools::executeDelete( dbConnection, req, pkValue );
if ( res == true )
_Cache::discard( pkValue );
else
{
// Simply ensure nothing was cached for this value if there's nothing to
// delete from DB
assert( _Cache::discard( pkValue ) == false );
}
return res;
}
static void clear()
......
......@@ -48,7 +48,7 @@ bool FsDiscoverer::discover( const std::string &entryPoint )
return false;
{
auto f = Folder::fetch( m_dbConn, entryPoint );
auto f = Folder::fromPath( m_dbConn, entryPoint );
// If the folder exists, we assume it is up to date
if ( f != nullptr )
return true;
......
......@@ -134,7 +134,7 @@ TEST_F( Labels, Delete )
auto labels = f->labels();
ASSERT_EQ( labels.size(), 2u );
ml->deleteLabel( "sea otter" );
ml->deleteLabel( l1 );
labels = f->labels();
ASSERT_EQ( labels.size(), 1u );
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment