[amarok] src: Meta::Album: introduce canUpdateCompilation(), setCompilation()

Matěj Laitl matej at laitl.cz
Tue May 1 19:02:37 UTC 2012


Git commit 59fa0ee6d3f2cbbe1298e74b7fc56f838712fa0e by Matěj Laitl.
Committed on 01/05/2012 at 18:14.
Pushed by laitl into branch 'master'.

Meta::Album: introduce canUpdateCompilation(), setCompilation()

and convert subclasses to use these. Many many subclasses already had
equivalent methods, this reduces code duplication and eliminates one
static_cast. Some subclasses had equivalent methods but never used
them, these are deleted in order to avoid dead code.

This is a prerequisite for next commit that unifies album actions
across collections. This is rather a bigger change, so I'm CCing
list - if anyone opposes, raise your voice.

CCMAIL: amarok-devel at kde.org

M  +0    -1    src/amarokurls/BookmarkTreeView.cpp
M  +1    -1    src/core-impl/collections/audiocd/AudioCdCollection.cpp
M  +13   -7    src/core-impl/collections/audiocd/AudioCdMeta.cpp
M  +3    -1    src/core-impl/collections/audiocd/AudioCdMeta.h
M  +0    -6    src/core-impl/collections/daap/DaapMeta.cpp
M  +0    -1    src/core-impl/collections/daap/DaapMeta.h
M  +8    -0    src/core-impl/collections/db/sql/SqlMeta.cpp
M  +2    -11   src/core-impl/collections/db/sql/SqlMeta.h
M  +13   -0    src/core-impl/collections/ipodcollection/IpodMeta.cpp
M  +3    -0    src/core-impl/collections/ipodcollection/IpodMeta.h
M  +19   -1    src/core-impl/collections/support/MemoryMeta.cpp
M  +14   -3    src/core-impl/collections/support/MemoryMeta.h
M  +0    -6    src/core-impl/collections/upnpcollection/UpnpMeta.cpp
M  +0    -1    src/core-impl/collections/upnpcollection/UpnpMeta.h
M  +0    -5    src/core-impl/meta/timecode/TimecodeMeta.cpp
M  +0    -1    src/core-impl/meta/timecode/TimecodeMeta.h
M  +13   -0    src/core/meta/Meta.h
M  +11   -6    src/services/ServiceMetaBase.cpp
M  +3    -1    src/services/ServiceMetaBase.h
M  +1    -1    src/services/amazon/AmazonParser.cpp

http://commits.kde.org/amarok/59fa0ee6d3f2cbbe1298e74b7fc56f838712fa0e

diff --git a/src/amarokurls/BookmarkTreeView.cpp b/src/amarokurls/BookmarkTreeView.cpp
index f5e8502..5c5261a 100644
--- a/src/amarokurls/BookmarkTreeView.cpp
+++ b/src/amarokurls/BookmarkTreeView.cpp
@@ -388,7 +388,6 @@ void BookmarkTreeView::slotCreateTimecodeTrack() const
     track->setGenre( genre );
 
     album->setAlbumArtist( artist );
-    album->setIsCompilation( false );
 
     //make the user give us some info about this item...
 
diff --git a/src/core-impl/collections/audiocd/AudioCdCollection.cpp b/src/core-impl/collections/audiocd/AudioCdCollection.cpp
index c0e9f8e..637c2d3 100644
--- a/src/core-impl/collections/audiocd/AudioCdCollection.cpp
+++ b/src/core-impl/collections/audiocd/AudioCdCollection.cpp
@@ -291,7 +291,7 @@ AudioCdCollection::infoFetchComplete( KJob *job )
                     trackPtr->setArtist( artistPtr );
                 else
                 {
-                    albumPtr->setIsCompilation( true );
+                    albumPtr->setCompilation( true );
 
                     Meta::AudioCdArtistPtr trackArtistPtr = Meta::AudioCdArtistPtr( new Meta::AudioCdArtist( trackArtist ) );
                     trackArtistPtr->addTrack( trackPtr );
diff --git a/src/core-impl/collections/audiocd/AudioCdMeta.cpp b/src/core-impl/collections/audiocd/AudioCdMeta.cpp
index 7ef9085..d767125 100644
--- a/src/core-impl/collections/audiocd/AudioCdMeta.cpp
+++ b/src/core-impl/collections/audiocd/AudioCdMeta.cpp
@@ -390,6 +390,19 @@ AudioCdAlbum::isCompilation() const
 }
 
 bool
+AudioCdAlbum::canUpdateCompilation() const
+{
+    return true;
+}
+
+void
+AudioCdAlbum::setCompilation( bool compilation )
+{
+    DEBUG_BLOCK
+    m_isCompilation = compilation;
+}
+
+bool
 AudioCdAlbum::hasAlbumArtist() const
 {
     return !m_albumArtist.isNull();
@@ -450,13 +463,6 @@ AudioCdAlbum::setAlbumArtist( AudioCdArtistPtr artist )
     m_albumArtist = artist;
 }
 
-void
-AudioCdAlbum::setIsCompilation( bool compilation )
-{
-    DEBUG_BLOCK
-    m_isCompilation = compilation;
-}
-
 //AudioCdGenre
 
 AudioCdGenre::AudioCdGenre( const QString &name )
diff --git a/src/core-impl/collections/audiocd/AudioCdMeta.h b/src/core-impl/collections/audiocd/AudioCdMeta.h
index 083a3e1..73ac547 100644
--- a/src/core-impl/collections/audiocd/AudioCdMeta.h
+++ b/src/core-impl/collections/audiocd/AudioCdMeta.h
@@ -164,6 +164,9 @@ class AudioCdAlbum : public Meta::Album
         virtual QString name() const;
 
         virtual bool isCompilation() const;
+        virtual bool canUpdateCompilation() const;
+        virtual void setCompilation( bool compilation );
+
         virtual bool hasAlbumArtist() const;
         virtual ArtistPtr albumArtist() const;
         virtual TrackList tracks();
@@ -176,7 +179,6 @@ class AudioCdAlbum : public Meta::Album
         //AudioCdAlbum specific methods
         void addTrack( AudioCdTrackPtr track );
         void setAlbumArtist( AudioCdArtistPtr artist );
-        void setIsCompilation( bool compilation );
 
     private:
         QString m_name;
diff --git a/src/core-impl/collections/daap/DaapMeta.cpp b/src/core-impl/collections/daap/DaapMeta.cpp
index e4e47b3..a2429d7 100644
--- a/src/core-impl/collections/daap/DaapMeta.cpp
+++ b/src/core-impl/collections/daap/DaapMeta.cpp
@@ -420,12 +420,6 @@ DaapAlbum::setAlbumArtist( DaapArtistPtr artist )
     m_albumArtist = artist;
 }
 
-void
-DaapAlbum::setIsCompilation( bool compilation )
-{
-    m_isCompilation = compilation;
-}
-
 //DaapGenre
 
 DaapGenre::DaapGenre( const QString &name )
diff --git a/src/core-impl/collections/daap/DaapMeta.h b/src/core-impl/collections/daap/DaapMeta.h
index dae7760..5278b57 100644
--- a/src/core-impl/collections/daap/DaapMeta.h
+++ b/src/core-impl/collections/daap/DaapMeta.h
@@ -167,7 +167,6 @@ class DaapAlbum : public Meta::Album
         //DaapAlbum specific methods
         void addTrack( DaapTrackPtr track );
         void setAlbumArtist( DaapArtistPtr artist );
-        void setIsCompilation( bool compilation );
 
     private:
         QString m_name;
diff --git a/src/core-impl/collections/db/sql/SqlMeta.cpp b/src/core-impl/collections/db/sql/SqlMeta.cpp
index 3413a75..da4d506 100644
--- a/src/core-impl/collections/db/sql/SqlMeta.cpp
+++ b/src/core-impl/collections/db/sql/SqlMeta.cpp
@@ -1893,6 +1893,14 @@ SqlAlbum::setImage( const QString &path )
     }
 }
 
+/** Set the compilation flag.
+ *  Actually it does not cange this album but instead moves
+ *  the tracks to other albums (e.g. one with the same name which is a
+ *  compilation)
+ *  If the compilation flag is set to "false" then all songs
+ *  with different artists will be moved to other albums, possibly even
+ *  creating them.
+ */
 void
 SqlAlbum::setCompilation( bool compilation )
 {
diff --git a/src/core-impl/collections/db/sql/SqlMeta.h b/src/core-impl/collections/db/sql/SqlMeta.h
index c0253ac..8ee1014 100644
--- a/src/core-impl/collections/db/sql/SqlMeta.h
+++ b/src/core-impl/collections/db/sql/SqlMeta.h
@@ -365,9 +365,9 @@ class AMAROK_SQLCOLLECTION_EXPORT_TESTS SqlAlbum : public Meta::Album
 
         virtual Meta::TrackList tracks();
 
-        /** Returns true if this album is a compilation, meaning that the included songs are from different artists.
-        */
         virtual bool isCompilation() const;
+        virtual bool canUpdateCompilation() const { return true; }
+        void setCompilation( bool compilation );
 
         /** Returns true if this album has an artist.
          *  The following equation is always true: isCompilation() != hasAlbumArtist()
@@ -411,15 +411,6 @@ class AMAROK_SQLCOLLECTION_EXPORT_TESTS SqlAlbum : public Meta::Album
         //SQL specific methods
         int id() const { return m_id; }
 
-        /** Set the compilation flag.
-         *  Actually it does not cange this album but instead moves
-         *  the tracks to other albums (e.g. one with the same name which is a
-         *  compilation)
-         *  If the compilation flag is set to "false" then all songs
-         *  with different artists will be moved to other albums, possibly even
-         *  creating them.
-         */
-        void setCompilation( bool compilation );
         Collections::SqlCollection *sqlCollection() const { return m_collection; }
 
     private:
diff --git a/src/core-impl/collections/ipodcollection/IpodMeta.cpp b/src/core-impl/collections/ipodcollection/IpodMeta.cpp
index 0f12bdb..dcb1431 100644
--- a/src/core-impl/collections/ipodcollection/IpodMeta.cpp
+++ b/src/core-impl/collections/ipodcollection/IpodMeta.cpp
@@ -770,6 +770,19 @@ Album::isCompilation() const
 }
 
 bool
+Album::canUpdateCompilation() const
+{
+    Collections::Collection *coll = m_track->collection();
+    return coll ? coll->isWritable() : false;
+}
+
+void
+Album::setCompilation( bool isCompilation )
+{
+    m_track->setIsCompilation( isCompilation );
+}
+
+bool
 Album::hasAlbumArtist() const
 {
     readAlbumArtist();
diff --git a/src/core-impl/collections/ipodcollection/IpodMeta.h b/src/core-impl/collections/ipodcollection/IpodMeta.h
index 2b14a9d..1df1342 100644
--- a/src/core-impl/collections/ipodcollection/IpodMeta.h
+++ b/src/core-impl/collections/ipodcollection/IpodMeta.h
@@ -258,6 +258,9 @@ namespace IpodMeta
             virtual Meta::TrackList tracks() { return Meta::TrackList(); }
 
             virtual bool isCompilation() const;
+            virtual bool canUpdateCompilation() const;
+            virtual void setCompilation( bool isCompilation );
+
             virtual bool hasAlbumArtist() const;
             virtual Meta::ArtistPtr albumArtist() const;
 
diff --git a/src/core-impl/collections/support/MemoryMeta.cpp b/src/core-impl/collections/support/MemoryMeta.cpp
index 8caa3dd..0332eda 100644
--- a/src/core-impl/collections/support/MemoryMeta.cpp
+++ b/src/core-impl/collections/support/MemoryMeta.cpp
@@ -49,12 +49,28 @@ Base::removeTrack( Track *track )
 Album::Album( const Meta::AlbumPtr &other )
     : Base( other->name() )
     , m_isCompilation( other->isCompilation() )
+    , m_canUpdateCompilation( other->canUpdateCompilation() )
     , m_image( other->image() )
 {
     if( other->hasAlbumArtist() && other->albumArtist() )
         m_albumArtist = Meta::ArtistPtr( new Artist( other->albumArtist()->name() ) );
 }
 
+void
+Album::setCompilation( bool isCompilation )
+{
+    // we re-create the albums for each track - that's how MemoryMeta works - by
+    // aggregating multiple entities of same name into one.
+    foreach( Meta::TrackPtr track, tracks() )
+    {
+        Track *memoryTrack = static_cast<Track *>( track.data() );
+        Meta::AlbumPtr album = memoryTrack->originalTrack()->album();
+        if( album && album->canUpdateCompilation() )
+            album->setCompilation( isCompilation );
+    }
+    // no notifyObservers() etc needed - this is done from down-up by proxied tracks
+}
+
 QImage
 Album::image( int size ) const
 {
@@ -204,9 +220,11 @@ MapChanger::addExistingTrack( Meta::TrackPtr track, Track *memoryTrack )
         m_mc->addAlbum( album );
     }
     bool isCompilation = trackAlbum ? trackAlbum->isCompilation() : false;
+    bool canUpdateCompilation = trackAlbum ? trackAlbum->canUpdateCompilation() : false;
     Album *memoryAlbum = static_cast<Album *>( album.data() );
     // be deterministic wrt track adding order:
-    memoryAlbum->setIsCompilation( memoryAlbum->isCompilation() || isCompilation );
+    memoryAlbum->setCachedCompilation( memoryAlbum->isCompilation() || isCompilation );
+    memoryAlbum->setCachedCanUpdateCompilation( memoryAlbum->canUpdateCompilation() || canUpdateCompilation );
     // optimisation: only read album cover if no previous one exists:
     if( !memoryAlbum->hasImage() && trackAlbum && trackAlbum->hasImage() )
             memoryAlbum->setImage( trackAlbum->image() );
diff --git a/src/core-impl/collections/support/MemoryMeta.h b/src/core-impl/collections/support/MemoryMeta.h
index 50b3d64..9ba1d1d 100644
--- a/src/core-impl/collections/support/MemoryMeta.h
+++ b/src/core-impl/collections/support/MemoryMeta.h
@@ -71,7 +71,11 @@ class Album : public Meta::Album, public Base
 {
     public:
         Album( const QString &name, const Meta::ArtistPtr &albumArtist )
-            : Base( name ), m_isCompilation( false ), m_albumArtist( albumArtist ) {}
+            : Base( name )
+            , m_isCompilation( false )
+            , m_canUpdateCompilation( false )
+            , m_albumArtist( albumArtist )
+            {}
         /**
          * Copy-like constructor for MapChanger
          */
@@ -79,8 +83,11 @@ class Album : public Meta::Album, public Base
 
         virtual QString name() const { return Base::name(); }
 
-        /** Meta::Album virtual methods */
+        /* Meta::Album virtual methods */
         virtual bool isCompilation() const { return m_isCompilation; }
+        virtual bool canUpdateCompilation() const { return m_canUpdateCompilation; }
+        virtual void setCompilation( bool isCompilation );
+
         virtual bool hasAlbumArtist() const { return !m_albumArtist.isNull(); }
         virtual Meta::ArtistPtr albumArtist() const { return m_albumArtist; }
         virtual Meta::TrackList tracks() { return Base::tracks(); }
@@ -92,10 +99,14 @@ class Album : public Meta::Album, public Base
         virtual void setImage( const QImage &image ) { m_image = image; }
 
         /* MemoryMeta::Album methods: */
-        void setIsCompilation( bool isCompilation ) { m_isCompilation = isCompilation; }
+        void setCachedCompilation( bool isCompilation )
+            { m_isCompilation = isCompilation; }
+        void setCachedCanUpdateCompilation( bool canUpdateCompilation )
+            { m_canUpdateCompilation = canUpdateCompilation; }
 
     private:
         bool m_isCompilation;
+        bool m_canUpdateCompilation;
         Meta::ArtistPtr m_albumArtist;
         QImage m_image;
 };
diff --git a/src/core-impl/collections/upnpcollection/UpnpMeta.cpp b/src/core-impl/collections/upnpcollection/UpnpMeta.cpp
index a2f4fc0..8d8bfb1 100644
--- a/src/core-impl/collections/upnpcollection/UpnpMeta.cpp
+++ b/src/core-impl/collections/upnpcollection/UpnpMeta.cpp
@@ -485,12 +485,6 @@ UpnpAlbum::setAlbumArtist( UpnpArtistPtr artist )
 }
 
 void
-UpnpAlbum::setIsCompilation( bool compilation )
-{
-    m_isCompilation = compilation;
-}
-
-void
 UpnpAlbum::setAlbumArtUrl( const KUrl &url )
 {
     m_albumArtUrl = url;
diff --git a/src/core-impl/collections/upnpcollection/UpnpMeta.h b/src/core-impl/collections/upnpcollection/UpnpMeta.h
index f1596b4..feabc1e 100644
--- a/src/core-impl/collections/upnpcollection/UpnpMeta.h
+++ b/src/core-impl/collections/upnpcollection/UpnpMeta.h
@@ -182,7 +182,6 @@ class UpnpAlbum : public QObject, public Meta::Album
         void addTrack( UpnpTrackPtr track );
         void removeTrack( UpnpTrackPtr track );
         void setAlbumArtist( UpnpArtistPtr artist );
-        void setIsCompilation( bool compilation );
         void setAlbumArtUrl( const KUrl &url );
 
     private:
diff --git a/src/core-impl/meta/timecode/TimecodeMeta.cpp b/src/core-impl/meta/timecode/TimecodeMeta.cpp
index 5ebe859..5d78d59 100644
--- a/src/core-impl/meta/timecode/TimecodeMeta.cpp
+++ b/src/core-impl/meta/timecode/TimecodeMeta.cpp
@@ -544,11 +544,6 @@ void TimecodeAlbum::setAlbumArtist( TimecodeArtistPtr artist )
     m_albumArtist = artist;
 }
 
-void TimecodeAlbum::setIsCompilation( bool compilation )
-{
-    m_isCompilation = compilation;
-}
-
 bool TimecodeAlbum::hasCapabilityInterface( Capabilities::Capability::Type type ) const
 {
     return type == Capabilities::Capability::Actions;
diff --git a/src/core-impl/meta/timecode/TimecodeMeta.h b/src/core-impl/meta/timecode/TimecodeMeta.h
index 7f24832..7ce2a67 100644
--- a/src/core-impl/meta/timecode/TimecodeMeta.h
+++ b/src/core-impl/meta/timecode/TimecodeMeta.h
@@ -201,7 +201,6 @@ public:
     //TimecodeAlbum specific methods
     void addTrack( TimecodeTrackPtr track );
     void setAlbumArtist( TimecodeArtistPtr artist );
-    void setIsCompilation( bool compilation );
 
     bool operator==( const Meta::Album &other ) const
     {
diff --git a/src/core/meta/Meta.h b/src/core/meta/Meta.h
index 2797a10..d028c4c 100644
--- a/src/core/meta/Meta.h
+++ b/src/core/meta/Meta.h
@@ -377,7 +377,20 @@ namespace Meta
 
             virtual QString prettyName() const;
 
+            /**
+             * Whether this album is considered to be a compilation of tracks from various
+             * artists.
+             */
             virtual bool isCompilation() const = 0;
+            /**
+             * Whether toggling the compilation status is currenlty supported. Default
+             * implementation returns false.
+             */
+            virtual bool canUpdateCompilation() const { return false; }
+            /**
+             * Set compilation status. You should check canUpdateCompilation() first.
+             */
+            virtual void setCompilation( bool isCompilation ) { Q_UNUSED( isCompilation ) }
 
             /** Returns true if this album has an album artist */
             virtual bool hasAlbumArtist() const = 0;
diff --git a/src/services/ServiceMetaBase.cpp b/src/services/ServiceMetaBase.cpp
index 9d585d1..039146b 100644
--- a/src/services/ServiceMetaBase.cpp
+++ b/src/services/ServiceMetaBase.cpp
@@ -735,6 +735,17 @@ ServiceAlbum::isCompilation() const
     return m_isCompilation;
 }
 
+bool ServiceAlbum::canUpdateCompilation() const
+{
+    return true;
+}
+
+void
+ServiceAlbum::setCompilation( bool compilation )
+{
+    m_isCompilation = compilation;
+}
+
 bool
 ServiceAlbum::hasAlbumArtist() const
 {
@@ -765,12 +776,6 @@ ServiceAlbum::setAlbumArtist( ArtistPtr artist )
     m_albumArtist = artist;
 }
 
-void
-ServiceAlbum::setIsCompilation( bool compilation )
-{
-    m_isCompilation = compilation;
-}
-
 void ServiceAlbum::setDescription(const QString &description)
 {
     m_description = description;
diff --git a/src/services/ServiceMetaBase.h b/src/services/ServiceMetaBase.h
index 195197e..793eb9e 100644
--- a/src/services/ServiceMetaBase.h
+++ b/src/services/ServiceMetaBase.h
@@ -363,6 +363,9 @@ class AMAROK_EXPORT ServiceAlbum : public Meta::Album,
         virtual QString name() const;
 
         virtual bool isCompilation() const;
+        virtual bool canUpdateCompilation() const;
+        virtual void setCompilation( bool compilation );
+
         virtual bool hasAlbumArtist() const;
         virtual ArtistPtr albumArtist() const;
         virtual TrackList tracks();
@@ -401,7 +404,6 @@ class AMAROK_EXPORT ServiceAlbum : public Meta::Album,
         //ServiceAlbum specific methods
         void addTrack( TrackPtr track );
         void setAlbumArtist( ArtistPtr artist );
-        void setIsCompilation( bool compilation );
 
         void setDescription( const QString &description );
         QString description( ) const;
diff --git a/src/services/amazon/AmazonParser.cpp b/src/services/amazon/AmazonParser.cpp
index 9988811..27ac3dc 100644
--- a/src/services/amazon/AmazonParser.cpp
+++ b/src/services/amazon/AmazonParser.cpp
@@ -192,7 +192,7 @@ AmazonParser::addAlbumToCollection( const QString &albumTitle, const QString &de
         results << albumID << albumTitle << description << artistID << price << imgUrl << albumAsin;
 
         Meta::AlbumPtr newAlbum = m_factory->createAlbum( results );
-        dynamic_cast<Meta::AmazonAlbum*>( newAlbum.data() )->setIsCompilation( isCompilation );
+        newAlbum->setCompilation( isCompilation );
         m_collection->addAlbum( newAlbum );
         m_collection->albumIDMap()->insert( albumAsin, albumID.toInt() );
     }



More information about the Amarok-devel mailing list