Media: Move metadata management in a specific class

Makefile.am

@@ -79,6 +79,7 @@ libmedialibrary_la_SOURCES = \
 	src/Thumbnail.cpp \
 	src/ShowEpisode.cpp \
 	src/VideoTrack.cpp \
+	src/Metadata.cpp \
 	src/database/SqliteConnection.cpp \
 	src/database/SqliteTools.cpp \
 	src/database/SqliteTransaction.cpp \
@@ -162,6 +163,7 @@ noinst_HEADERS = \
 	src/utils/SWMRLock.h \
 	src/utils/Url.h \
 	src/VideoTrack.h \
+	src/Metadata.h \
 	src/compat/Thread.h \
 	src/compat/Mutex.h \
 	src/compat/ConditionVariable.h \

src/Media.cpp

@@ -57,10 +57,9 @@ const std::string policy::MediaTable::Name = "Media";
 const std::string policy::MediaTable::PrimaryKeyColumn = "id_media";
 int64_t Media::* const policy::MediaTable::PrimaryKey = &Media::m_id;
 
-const std::string policy::MediaMetadataTable::Name = "MediaMetadata";
-
 Media::Media( MediaLibraryPtr ml, sqlite::Row& row )
     : m_ml( ml )
+    , m_metadata( m_ml )
     , m_changed( false )
 {
     row >> m_id
@@ -96,6 +95,7 @@ Media::Media( MediaLibraryPtr ml, const std::string& title, Type type )
     , m_filename( title )
     , m_isFavorite( false )
     , m_isPresent( true )
+    , m_metadata( m_ml )
     , m_changed( false )
 {
 }
@@ -300,72 +300,23 @@ unsigned int Media::releaseDate() const
 
 const IMediaMetadata& Media::metadata( IMedia::MetadataType type ) const
 {
-    auto lock = m_metadata.lock();
-    if ( m_metadata.isCached() == false )
-    {
-        std::vector<MediaMetadata> res;
-        // Reserve the space for all meta to avoid a race condition where 2 threads
-        // would cache different meta, invalidating the potential reference
-        // to another IMediaMetadata held by another thread.
-        // This guarantees the vector will not grow afterward.
-        res.reserve( IMedia::NbMeta );
-        static const std::string req = "SELECT * FROM " + policy::MediaMetadataTable::Name +
-                " WHERE id_media = ?";
-        auto conn = m_ml->getConn();
-        auto ctx = conn->acquireReadContext();
-        sqlite::Statement stmt( conn->handle(), req );
-        stmt.execute( m_id );
-        for ( sqlite::Row row = stmt.row(); row != nullptr; row = stmt.row() )
-        {
-            assert( row.load<int64_t>( 0 ) == m_id );
-            res.emplace_back( row.load<decltype(MediaMetadata::m_type)>( 1 ),
-                              row.load<decltype(MediaMetadata::m_value)>( 2 ) );
-        }
-        m_metadata = std::move( res );
-    }
-    auto it = std::find_if( begin( m_metadata.get() ), end( m_metadata.get() ), [type](const MediaMetadata& m ) {
-        return m.m_type == type;
-    });
-    if ( it == end( m_metadata.get() ) )
-    {
-        m_metadata.get().emplace_back( type );
-        return *m_metadata.get().rbegin();
-    }
-    return (*it);
+    if ( m_metadata.isReady() == false )
+        m_metadata.init( m_id, IMedia::NbMeta );
+    return m_metadata.get( type );
 }
 
 bool Media::setMetadata( IMedia::MetadataType type, const std::string& value )
 {
-    {
-        auto lock = m_metadata.lock();
-        if ( m_metadata.isCached() == true )
-        {
-            auto it = std::find_if( begin( m_metadata.get() ), end( m_metadata.get() ), [type](const MediaMetadata& m ) {
-                return m.m_type == type;
-            });
-            if ( it != end( m_metadata.get() ) )
-                (*it).set( value );
-            else
-                m_metadata.get().emplace_back( type, value );
-        }
-    }
-    try
-    {
-        static const std::string req = "INSERT OR REPLACE INTO " + policy::MediaMetadataTable::Name +
-                "(id_media, type, value) VALUES(?, ?, ?)";
-        return sqlite::Tools::executeInsert( m_ml->getConn(), req, m_id, type, value );
-    }
-    catch ( const sqlite::errors::Generic& ex )
-    {
-        LOG_ERROR( "Failed to update media metadata: ", ex.what() );
-        return false;
-    }
+    if ( m_metadata.isReady() == false )
+        m_metadata.init( m_id, IMedia::NbMeta );
+    return m_metadata.set( type, value );
 }
 
 bool Media::setMetadata( IMedia::MetadataType type, int64_t value )
 {
-    auto str = std::to_string( value );
-    return setMetadata( type, str );
+    if ( m_metadata.isReady() == false )
+        m_metadata.init( m_id, IMedia::NbMeta );
+    return m_metadata.set( type, value );
 }
 
 void Media::setReleaseDate( unsigned int date )
@@ -604,15 +555,8 @@ void Media::createTable( sqlite::Connection* connection )
                 "title,"
                 "labels"
             ")";
-    const std::string metadataReq = "CREATE TABLE IF NOT EXISTS " + policy::MediaMetadataTable::Name + "("
-            "id_media INTEGER,"
-            "type INTEGER,"
-            "value TEXT,"
-            "PRIMARY KEY (id_media, type)"
-            ")";
     sqlite::Tools::executeRequest( connection, req );
     sqlite::Tools::executeRequest( connection, vtableReq );
-    sqlite::Tools::executeRequest( connection, metadataReq );
 }
 
 void Media::createTriggers( sqlite::Connection* connection )
@@ -759,25 +703,4 @@ void Media::clearHistory( MediaLibraryPtr ml )
     sqlite::Tools::executeDelete( dbConn, flushProgress, IMedia::MetadataType::Progress );
 }
 
-bool Media::MediaMetadata::isSet() const
-{
-    return m_isSet;
-}
-
-int64_t Media::MediaMetadata::integer() const
-{
-    return atoll( m_value.c_str() );
-}
-
-const std::string& Media::MediaMetadata::str() const
-{
-    return m_value;
-}
-
-void Media::MediaMetadata::set( const std::string& value )
-{
-    m_value = value;
-    m_isSet = true;
-}
-
 }

src/Media.h

@@ -31,6 +31,7 @@
 #include "database/DatabaseHelpers.h"
 #include "utils/Cache.h"
 #include "medialibrary/IMetadata.h"
+#include "Metadata.h"
 
 namespace medialibrary
 {
@@ -51,33 +52,10 @@ struct MediaTable
     static int64_t Media::*const PrimaryKey;
 };
 
-struct MediaMetadataTable
-{
-    static const std::string Name;
-};
 }
 
 class Media : public IMedia, public DatabaseHelpers<Media, policy::MediaTable>
 {
-    class MediaMetadata : public IMediaMetadata
-    {
-    public:
-        MediaMetadata(MetadataType t, std::string v) : m_type( t ), m_value( std::move( v ) ), m_isSet( true ) {}
-        MediaMetadata(MetadataType t) : m_type( t ), m_isSet( false ) {}
-        virtual bool isSet() const override;
-        virtual int64_t integer() const override;
-        virtual const std::string& str() const override;
-
-    private:
-        void set( const std::string& value );
-
-    private:
-        MetadataType m_type;
-        std::string m_value;
-        bool m_isSet;
-        friend class Media;
-    };
-
     public:
         // Those should be private, however the standard states that the expression
         // ::new (pv) T(std::forward(args)...)
@@ -181,7 +159,7 @@ private:
         mutable Cache<ShowEpisodePtr> m_showEpisode;
         mutable Cache<MoviePtr> m_movie;
         mutable Cache<std::vector<FilePtr>> m_files;
-        mutable Cache<std::vector<MediaMetadata>> m_metadata;
+        mutable MediaMetadata m_metadata;
         mutable Cache<std::shared_ptr<Thumbnail>> m_thumbnail;
         bool m_changed;
 

src/MediaLibrary.cpp

@@ -58,6 +58,7 @@
 #include "utils/Filename.h"
 #include "utils/Url.h"
 #include "VideoTrack.h"
+#include "Metadata.h"
 
 // Discoverers:
 #include "discoverer/FsDiscoverer.h"
@@ -168,6 +169,7 @@ void MediaLibrary::createAllTables()
     History::createTable( m_dbConnection.get() );
     Settings::createTable( m_dbConnection.get() );
     parser::Task::createTable( m_dbConnection.get() );
+    MediaMetadata::createTable( m_dbConnection.get() );
 }
 
 void MediaLibrary::createAllTriggers()

src/Metadata.cpp

+/*****************************************************************************
+ * Media Library
+ *****************************************************************************
+ * Copyright (C) 2015-2018 Hugo Beauzée-Luyssen, Videolabs, VideoLAN
+ *
+ * Authors: Hugo Beauzée-Luyssen <hugo@beauzee.fr>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
+ *****************************************************************************/
+
+#ifdef HAVE_CONFIG_H
+# include "config.h"
+#endif
+
+#include "Metadata.h"
+
+#include "database/SqliteTools.h"
+
+#include <algorithm>
+
+namespace medialibrary
+{
+
+const std::string policy::MediaMetadataTable::Name = "MediaMetadata";
+
+MediaMetadata::Record::Record( IMedia::MetadataType t, std::string v )
+    : m_type( t )
+    , m_value( std::move( v ) )
+    , m_isSet( true )
+{
+}
+
+MediaMetadata::Record::Record( IMedia::MetadataType t )
+    : m_type( t )
+    , m_isSet( false )
+{
+}
+
+bool MediaMetadata::Record::isSet() const
+{
+    return m_isSet;
+}
+
+int64_t MediaMetadata::Record::integer() const
+{
+    return atoll( m_value.c_str() );
+}
+
+const std::string& MediaMetadata::Record::str() const
+{
+    return m_value;
+}
+
+MediaMetadata::MediaMetadata( MediaLibraryPtr ml )
+    : m_ml( ml )
+    , m_nbMeta( 0 )
+    , m_entityId( 0 )
+{
+}
+
+void MediaMetadata::init( int64_t entityId, uint32_t nbMeta )
+{
+    if ( isReady() == true )
+        return;
+
+    m_nbMeta = nbMeta;
+    m_entityId = entityId;
+    // Reserve the space for all meta to avoid a race condition where 2 threads
+    // would cache different meta, invalidating the potential reference
+    // to another IMediaMetadata held by another thread.
+    // This guarantees the vector will not grow afterward.
+    m_records.reserve( m_nbMeta );
+    static const std::string req = "SELECT * FROM " + policy::MediaMetadataTable::Name +
+            " WHERE id_media = ?";
+    auto conn = m_ml->getConn();
+    auto ctx = conn->acquireReadContext();
+    sqlite::Statement stmt( conn->handle(), req );
+    stmt.execute( m_entityId );
+    for ( sqlite::Row row = stmt.row(); row != nullptr; row = stmt.row() )
+    {
+        assert( row.load<int64_t>( 0 ) == m_entityId );
+        m_records.emplace_back( row.load<decltype(Record::m_type)>( 1 ),
+                          row.load<decltype(Record::m_value)>( 2 ) );
+    }
+}
+
+bool MediaMetadata::isReady() const
+{
+    return m_nbMeta != 0;
+}
+
+IMediaMetadata&MediaMetadata::get( IMedia::MetadataType type ) const
+{
+    assert( isReady() == true );
+
+    auto it = std::find_if( begin( m_records ), end( m_records ), [type](const Record& r ) {
+        return r.m_type == type;
+    });
+    if ( it == end( m_records ) )
+    {
+        // Create an unset meta for the given type˙No DB entity will be created until
+        // the meta is actually set.
+        m_records.emplace_back( type );
+        return *( m_records.rbegin() );
+    }
+    return *it;
+}
+
+bool MediaMetadata::set( IMedia::MetadataType type, const std::string& value )
+{
+    assert( isReady() == true );
+
+    auto it = std::find_if( begin( m_records ), end( m_records ), [type]( const Record& r ) {
+        return r.m_type == type;
+    });
+    if ( it != end( m_records ) )
+        (*it).set( value );
+    else
+        m_records.emplace_back( type, value );
+    try
+    {
+        static const std::string req = "INSERT OR REPLACE INTO " + policy::MediaMetadataTable::Name +
+                "(id_media, type, value) VALUES(?, ?, ?)";
+        return sqlite::Tools::executeInsert( m_ml->getConn(), req, m_entityId, type, value );
+    }
+    catch ( const sqlite::errors::Generic& ex )
+    {
+        LOG_ERROR( "Failed to update media metadata: ", ex.what() );
+        return false;
+    }
+}
+
+bool MediaMetadata::set( IMedia::MetadataType type, int64_t value )
+{
+    auto str = std::to_string( value );
+    return set( type, str );
+}
+
+void MediaMetadata::createTable(sqlite::Connection* connection)
+{
+    const std::string req = "CREATE TABLE IF NOT EXISTS " + policy::MediaMetadataTable::Name + "("
+            "id_media INTEGER,"
+            "type INTEGER,"
+            "value TEXT,"
+            "PRIMARY KEY (id_media, type)"
+            ")";
+    sqlite::Tools::executeRequest( connection, req );
+}
+
+void MediaMetadata::Record::set( const std::string& value )
+{
+    m_value = value;
+    m_isSet = true;
+}
+
+}

src/Metadata.h

+/*****************************************************************************
+ * Media Library
+ *****************************************************************************
+ * Copyright (C) 2015-2018 Hugo Beauzée-Luyssen, Videolabs, VideoLAN
+ *
+ * Authors: Hugo Beauzée-Luyssen <hugo@beauzee.fr>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
+ *****************************************************************************/
+
+#pragma once
+
+#include "medialibrary/IMetadata.h"
+#include "medialibrary/IMedia.h"
+#include "Types.h"
+
+namespace medialibrary
+{
+
+namespace sqlite
+{
+class Connection;
+}
+
+namespace policy
+{
+struct MediaMetadataTable
+{
+    static const std::string Name;
+};
+}
+
+class MediaMetadata
+{
+public:
+    MediaMetadata( MediaLibraryPtr ml );
+
+    // We have to "lazy init" this object since during containing object creation,
+    // we might not know the ID yet (for instance. when instantiating the
+    // metadata object during the creation of a new Media)
+    void init( int64_t entityId, uint32_t nbMeta );
+    bool isReady() const;
+
+    IMediaMetadata& get( IMedia::MetadataType type ) const;
+    bool set( IMedia::MetadataType type, const std::string& value );
+    bool set( IMedia::MetadataType type, int64_t value );
+
+    static void createTable( sqlite::Connection* connection );
+
+private:
+    class Record : public IMediaMetadata
+    {
+    public:
+        virtual ~Record() = default;
+        Record( IMedia::MetadataType t, std::string v );
+        Record( IMedia::MetadataType t );
+        virtual bool isSet() const override;
+        virtual int64_t integer() const override;
+        virtual const std::string& str() const override;
+        void set( const std::string& value );
+
+    private:
+        IMedia::MetadataType m_type;
+        std::string m_value;
+        bool m_isSet;
+
+        friend MediaMetadata;
+    };
+
+private:
+    MediaLibraryPtr m_ml;
+    uint32_t m_nbMeta;
+    int64_t m_entityId;
+    mutable std::vector<Record> m_records;
+};
+
+}