From 30eaf90c6aed98db194a2eb2b4b1b4014afcde6b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hugo=20Beauz=C3=A9e-Luyssen?= <hugo@beauzee.fr>
Date: Sat, 17 May 2014 17:12:24 +0300
Subject: [PATCH] Rework File/Label API to make it more natural

---
 include/IFile.h         |  9 +++--
 include/ILabel.h        |  6 +--
 include/IMediaLibrary.h |  3 ++
 src/File.cpp            | 90 ++++++++++++++++++++++-------------------
 src/File.h              |  7 ++--
 src/Label.cpp           | 43 +-------------------
 src/Label.h             |  4 +-
 src/MediaLibrary.cpp    | 10 +++++
 src/MediaLibrary.h      |  2 +-
 test/Tests.cpp          | 24 ++++++++---
 10 files changed, 92 insertions(+), 106 deletions(-)

diff --git a/include/IFile.h b/include/IFile.h
index 2e4e5de09..a8d1ef5c0 100644
--- a/include/IFile.h
+++ b/include/IFile.h
@@ -3,10 +3,11 @@
 
 #include <vector>
 #include <memory>
+
+#include "IMediaLibrary.h"
 #include "ITrackInformation.h"
 
 class IAlbumTrack;
-class ILabel;
 class IShowEpisode;
 class ITrackInformation;
 
@@ -21,9 +22,9 @@ class IFile
         virtual std::shared_ptr<IShowEpisode> showEpisode() = 0;
         virtual int playCount() = 0;
         virtual const std::string& mrl() = 0;
-        virtual std::shared_ptr<ILabel> addLabel( const std::string& label ) = 0;
-        virtual bool removeLabel( const std::shared_ptr<ILabel>& label ) = 0;
-        virtual const std::vector<std::shared_ptr<ILabel>>& labels() = 0;
+        virtual bool addLabel( LabelPtr label ) = 0;
+        virtual bool removeLabel( LabelPtr label ) = 0;
+        virtual std::vector<std::shared_ptr<ILabel> > labels() = 0;
 };
 
 #endif // IFILE_H
diff --git a/include/ILabel.h b/include/ILabel.h
index 0f27bb5cf..93f9b3575 100644
--- a/include/ILabel.h
+++ b/include/ILabel.h
@@ -4,7 +4,7 @@
 #include <memory>
 #include <vector>
 
-class IFile;
+#include "IMediaLibrary.h"
 
 class ILabel
 {
@@ -13,9 +13,7 @@ class ILabel
 
         virtual unsigned int id() const = 0;
         virtual const std::string& name() = 0;
-        virtual std::vector<std::shared_ptr<IFile>>& files() = 0;
-        virtual bool link( IFile* file ) = 0;
-        virtual bool unlink( IFile* file ) const = 0;
+        virtual std::vector<FilePtr>& files() = 0;
 };
 
 #endif // ILABEL_H
diff --git a/include/IMediaLibrary.h b/include/IMediaLibrary.h
index 0e26e39dd..c3a7d4a26 100644
--- a/include/IMediaLibrary.h
+++ b/include/IMediaLibrary.h
@@ -6,8 +6,10 @@
 #include <memory>
 
 class IFile;
+class ILabel;
 
 typedef std::shared_ptr<IFile> FilePtr;
+typedef std::shared_ptr<ILabel> LabelPtr;
 
 class IMediaLibrary
 {
@@ -16,6 +18,7 @@ class IMediaLibrary
         virtual bool initialize( const std::string& dbPath ) = 0;
         virtual FilePtr addFile( const std::string& path ) = 0;
         virtual FilePtr file( const std::string& path ) = 0;
+        virtual LabelPtr createLabel( const std::string& label ) = 0;
         virtual bool files( std::vector<FilePtr>& res ) = 0;
 };
 
diff --git a/src/File.cpp b/src/File.cpp
index 3c598b348..1d1f12437 100644
--- a/src/File.cpp
+++ b/src/File.cpp
@@ -14,7 +14,6 @@ const std::string policy::FileTable::CacheColumn = "mrl";
 
 File::File( sqlite3* dbConnection, sqlite3_stmt* stmt )
     : m_dbConnection( dbConnection )
-    , m_labels( nullptr )
 {
     m_id = sqlite3_column_int( stmt, 0 );
     m_type = (Type)sqlite3_column_int( stmt, 1 );
@@ -34,7 +33,6 @@ File::File( const std::string& mrl )
     , m_playCount( 0 )
     , m_showEpisodeId( 0 )
     , m_mrl( mrl )
-    , m_labels( nullptr )
 {
 }
 
@@ -87,17 +85,14 @@ std::shared_ptr<IShowEpisode> File::showEpisode()
     return m_showEpisode;
 }
 
-const std::vector<std::shared_ptr<ILabel>>& File::labels()
+std::vector<std::shared_ptr<ILabel> > File::labels()
 {
-    if ( m_labels == nullptr )
-    {
-        m_labels = new std::vector<std::shared_ptr<ILabel>>;
-        const char* req = "SELECT l.* FROM Label l "
-                "LEFT JOIN LabelFileRelation lfr ON lfr.id_label = l.id_label "
-                "WHERE lfr.id_file = ?";
-        SqliteTools::fetchAll<Label>( m_dbConnection, req, m_id, *m_labels );
-    }
-    return *m_labels;
+    std::vector<std::shared_ptr<ILabel> > labels;
+    const char* req = "SELECT l.* FROM Label l "
+            "LEFT JOIN LabelFileRelation lfr ON lfr.id_label = l.id_label "
+            "WHERE lfr.id_file = ?";
+    SqliteTools::fetchAll<Label>( m_dbConnection, req, m_id, labels );
+    return labels;
 }
 
 int File::playCount()
@@ -110,36 +105,6 @@ const std::string& File::mrl()
     return m_mrl;
 }
 
-std::shared_ptr<ILabel> File::addLabel(const std::string& label)
-{
-    auto l = Label::create( label );
-    if ( l->insert( m_dbConnection ) == false )
-    {
-        return nullptr;
-    }
-    l->link( this );
-    return l;
-}
-
-bool File::removeLabel( const std::shared_ptr<ILabel>& label )
-{
-    if ( m_labels != false )
-    {
-        auto it = m_labels->begin();
-        auto ite = m_labels->end();
-        while ( it != ite )
-        {
-            if ( (*it)->id() == label->id() )
-                break;
-            ++it;
-        }
-        if ( it == ite )
-            return false;
-        m_labels->erase( it );
-    }
-    return label->unlink( this );
-}
-
 unsigned int File::id() const
 {
     return m_id;
@@ -160,6 +125,47 @@ bool File::createTable(sqlite3* connection)
     return SqliteTools::createTable( connection, req );
 }
 
+bool File::addLabel( LabelPtr label )
+{
+    if ( m_dbConnection == nullptr || m_id == 0 || label->id() == 0)
+    {
+        std::cerr << "Both file & label need to be inserted in database before being linked together" << std::endl;
+        return false;
+    }
+    const char* req = "INSERT INTO LabelFileRelation VALUES(?, ?)";
+    sqlite3_stmt* stmt;
+    if ( sqlite3_prepare_v2( m_dbConnection, req, -1, &stmt, NULL ) != SQLITE_OK )
+    {
+        std::cerr << "Failed to insert record: " << sqlite3_errmsg( m_dbConnection ) << std::endl;
+        return false;
+    }
+    sqlite3_bind_int( stmt, 1, label->id() );
+    sqlite3_bind_int( stmt, 2, m_id );
+    bool res = sqlite3_step( stmt ) == SQLITE_DONE;
+    sqlite3_finalize( stmt );
+    return res;
+}
+
+bool File::removeLabel( LabelPtr label )
+{
+    if ( m_dbConnection == nullptr || m_id == 0 || label->id() == 0 )
+    {
+        std::cerr << "Can't unlink a label/file not inserted in database" << std::endl;
+        return false;
+    }
+    const char* req = "DELETE FROM LabelFileRelation WHERE id_label = ? AND id_file = ?";
+    sqlite3_stmt* stmt;
+    if ( sqlite3_prepare_v2( m_dbConnection, req, -1, &stmt, NULL ) != SQLITE_OK )
+    {
+        std::cerr << "Failed to remove record: " << sqlite3_errmsg( m_dbConnection ) << std::endl;
+        return false;
+    }
+    sqlite3_bind_int( stmt, 1, label->id() );
+    sqlite3_bind_int( stmt, 2, m_id );
+    sqlite3_step( stmt );
+    sqlite3_finalize( stmt );
+    return sqlite3_changes( m_dbConnection ) > 0;
+}
 
 const std::string& policy::FileCache::key(const std::shared_ptr<File> self )
 {
diff --git a/src/File.h b/src/File.h
index 223089010..fb49995d2 100644
--- a/src/File.h
+++ b/src/File.h
@@ -56,11 +56,11 @@ class File : public IFile, public Cache<File, IFile, policy::FileTable, policy::
         virtual std::shared_ptr<IAlbumTrack> albumTrack();
         virtual unsigned int duration();
         virtual std::shared_ptr<IShowEpisode> showEpisode();
-        virtual const std::vector<std::shared_ptr<ILabel>>& labels();
+        virtual bool addLabel( LabelPtr label );
+        virtual bool removeLabel( LabelPtr label );
+        virtual std::vector<std::shared_ptr<ILabel> > labels();
         virtual int playCount();
         virtual const std::string& mrl();
-        virtual std::shared_ptr<ILabel> addLabel( const std::string &label );
-        virtual bool removeLabel(const std::shared_ptr<ILabel>& label );
 
     private:
         sqlite3* m_dbConnection;
@@ -78,7 +78,6 @@ class File : public IFile, public Cache<File, IFile, policy::FileTable, policy::
         Album* m_album;
         std::shared_ptr<AlbumTrack> m_albumTrack;
         std::shared_ptr<ShowEpisode> m_showEpisode;
-        std::vector<std::shared_ptr<ILabel>>* m_labels;
 
         friend class Cache<File, IFile, policy::FileTable, policy::FileCache>;
 };
diff --git a/src/Label.cpp b/src/Label.cpp
index 274476401..79bd7a5d8 100644
--- a/src/Label.cpp
+++ b/src/Label.cpp
@@ -36,7 +36,7 @@ const std::string& Label::name()
     return m_name;
 }
 
-std::vector<std::shared_ptr<IFile>>& Label::files()
+std::vector<FilePtr>& Label::files()
 {
     if ( m_files == nullptr )
     {
@@ -86,44 +86,3 @@ bool Label::createTable(sqlite3* dbConnection)
     return SqliteTools::createTable( dbConnection, req );
 }
 
-bool Label::link( IFile* file )
-{
-    if ( m_dbConnection == nullptr || m_id == 0 )
-    {
-        std::cerr << "A label needs to be inserted in database before being linked to a file" << std::endl;
-        return false;
-    }
-    const char* req = "INSERT INTO LabelFileRelation VALUES(?, ?)";
-    sqlite3_stmt* stmt;
-    if ( sqlite3_prepare_v2( m_dbConnection, req, -1, &stmt, NULL ) != SQLITE_OK )
-    {
-        std::cerr << "Failed to insert record: " << sqlite3_errmsg( m_dbConnection ) << std::endl;
-        return false;
-    }
-    sqlite3_bind_int( stmt, 1, m_id );
-    sqlite3_bind_int( stmt, 2, file->id() );
-    bool res = sqlite3_step( stmt ) == SQLITE_DONE;
-    sqlite3_finalize( stmt );
-    return res;
-}
-
-bool Label::unlink( IFile* file ) const
-{
-    if ( m_dbConnection == nullptr || m_id == 0 )
-    {
-        std::cerr << "Can't unlink a label not inserted in database" << std::endl;
-        return false;
-    }
-    const char* req = "DELETE FROM LabelFileRelation WHERE id_label = ? AND id_file = ?";
-    sqlite3_stmt* stmt;
-    if ( sqlite3_prepare_v2( m_dbConnection, req, -1, &stmt, NULL ) != SQLITE_OK )
-    {
-        std::cerr << "Failed to remove record: " << sqlite3_errmsg( m_dbConnection ) << std::endl;
-        return false;
-    }
-    sqlite3_bind_int( stmt, 1, m_id );
-    sqlite3_bind_int( stmt, 2, file->id() );
-    bool res = sqlite3_step( stmt ) == SQLITE_DONE;
-    sqlite3_finalize( stmt );
-    return res;
-}
diff --git a/src/Label.h b/src/Label.h
index 626a9ba29..9ecb18d54 100644
--- a/src/Label.h
+++ b/src/Label.h
@@ -28,12 +28,10 @@ class Label : public ILabel, public Cache<Label, ILabel, policy::LabelTable>
     public:
         virtual unsigned int id() const;
         virtual const std::string& name();
-        virtual std::vector<std::shared_ptr<IFile>>& files();
+        virtual std::vector<FilePtr>& files();
         bool insert( sqlite3* dbConnection );
 
         static bool createTable( sqlite3* dbConnection );
-        bool link( IFile* file );
-        bool unlink( IFile* file ) const;
     private:
         sqlite3* m_dbConnection;
         unsigned int m_id;
diff --git a/src/MediaLibrary.cpp b/src/MediaLibrary.cpp
index 570d89d34..10c4e8457 100644
--- a/src/MediaLibrary.cpp
+++ b/src/MediaLibrary.cpp
@@ -50,3 +50,13 @@ FilePtr MediaLibrary::addFile( const std::string& path )
     }
     return f;
 }
+
+LabelPtr MediaLibrary::createLabel( const std::string& label )
+{
+    auto l = Label::create( label );
+    if ( l->insert( m_dbConnection ) == false )
+    {
+        return nullptr;
+    }
+    return l;
+}
diff --git a/src/MediaLibrary.h b/src/MediaLibrary.h
index 8a3fa4614..28bf91ff3 100644
--- a/src/MediaLibrary.h
+++ b/src/MediaLibrary.h
@@ -13,9 +13,9 @@ class MediaLibrary : public IMediaLibrary
         virtual bool files( std::vector<FilePtr>& res );
         virtual FilePtr file( const std::string& path );
         virtual FilePtr addFile( const std::string& path );
+        virtual LabelPtr createLabel(const std::string& label);
 
     private:
         sqlite3* m_dbConnection;
 };
-
 #endif // MEDIALIBRARY_H
diff --git a/test/Tests.cpp b/test/Tests.cpp
index 6e2ef2128..ced8b0c1b 100644
--- a/test/Tests.cpp
+++ b/test/Tests.cpp
@@ -59,13 +59,20 @@ TEST_F( MLTest, FetchFile )
 TEST_F( MLTest, AddLabel )
 {
     auto f = ml->addFile( "/dev/null" );
-    auto l1 = f->addLabel( "sea otter" );
-    auto l2 = f->addLabel( "cony the cone" );
+    auto l1 = ml->createLabel( "sea otter" );
+    auto l2 = ml->createLabel( "cony the cone" );
 
-    ASSERT_TRUE( l1 != NULL );
-    ASSERT_TRUE( l2 != NULL );
+    ASSERT_NE( l1, nullptr);
+    ASSERT_NE( l2, nullptr);
 
     auto labels = f->labels();
+    ASSERT_EQ( labels.size(), 0u );
+
+    f->addLabel( l1 );
+    f->addLabel( l2 );
+
+    labels = f->labels();
+
     ASSERT_EQ( labels.size(), 2u );
     ASSERT_EQ( labels[0]->name(), "sea otter" );
     ASSERT_EQ( labels[1]->name(), "cony the cone" );
@@ -74,8 +81,11 @@ TEST_F( MLTest, AddLabel )
 TEST_F( MLTest, RemoveLabel )
 {
     auto f = ml->addFile( "/dev/null" );
-    auto l1 = f->addLabel( "sea otter" );
-    auto l2 = f->addLabel( "cony the cone" );
+    auto l1 = ml->createLabel( "sea otter" );
+    auto l2 = ml->createLabel( "cony the cone" );
+
+    f->addLabel( l1 );
+    f->addLabel( l2 );
 
     auto labels = f->labels();
     ASSERT_EQ( labels.size(), 2u );
@@ -110,3 +120,5 @@ TEST_F( MLTest, RemoveLabel )
     labels = f2->labels();
     ASSERT_EQ( labels.size(), 0u );
 }
+
+
-- 
GitLab