[Amarok] 9c6c321 After a move operation startRemoveWorkflow will be

[Casey Link]

commit 9c6c321d924e748fe77052b21d809efb296363f5
Author: Casey Link <unnamedrambler at gmail.com>
Date:   Sat Feb 6 12:48:09 2010 -0500

    After a move operation startRemoveWorkflow will be used to remove
    succesfully moved tracks from the source collection.
    A result of this change is that SqlCollectionLocation::remove()
    only removes tracks from the database which do NOT exist on disk.
    The actual deletion from disk occurs earlier in the Remove Workflow
    using async KIO delete jobs.
    This way when things go wrong (e.g., no write perms) the state of
    tracks on disk will always match the collection database.
    
    Big note: when you want to remove a track ALWAYS use the full remove
    workflow, and do NOT call CollectionLocation::remove() directly.
    
    CCMAIL: amarok-devel at kde.org

diff --git a/src/collection/CollectionLocation.cpp b/src/collection/CollectionLocation.cpp
index 26a7d03..7ea1272 100644
--- a/src/collection/CollectionLocation.cpp
+++ b/src/collection/CollectionLocation.cpp
@@ -381,7 +381,6 @@ void
 CollectionLocation::slotFinishRemove()
 {
     DEBUG_BLOCK
-    removeSourceTracks( m_tracksSuccessfullyTransferred );
     if( m_tracksWithError.size() > 0 )
     {
         QStringList files;
@@ -513,40 +512,19 @@ void
 CollectionLocation::removeSourceTracks( const Meta::TrackList &tracks )
 {
     DEBUG_BLOCK
-    Meta::TrackList notDeletableTracks;
-    int count = m_tracksWithError.count();
-    debug() << "Transfer errors: " << count;
-    foreach( Meta::TrackPtr track, tracks )
-    {
-        if( m_tracksWithError.contains( track ) )
-        {
-            debug() << "transfer error for track " << track->playableUrl();
-            continue;
-        }
+    debug() << "Transfer errors: " << m_tracksWithError.count();
 
-        if( !remove( track ) )
-        {
-            debug() << "remove failed";
-            notDeletableTracks.append( track );
-        }
+    foreach( Meta::TrackPtr track, m_tracksWithError.keys() )
+    {
+        debug() << "transfer error for track " << track->playableUrl();
     }
 
-    if( notDeletableTracks.size() > 0 )
-    {
-        QStringList files;
-        foreach( Meta::TrackPtr track, notDeletableTracks )
-        {
-            if(track)
-                files << track->prettyUrl();
-        }
+    QSet<Meta::TrackPtr> toRemove = QSet<Meta::TrackPtr>::fromList( tracks );
+    QSet<Meta::TrackPtr> errored = QSet<Meta::TrackPtr>::fromList( m_tracksWithError.keys() );
+    toRemove.subtract( errored );
 
-        const QString text( i18ncp( "@info", "There was a problem and this track could not be removed. Make sure the directory is writeable.",
-                                    "There was a problem and %1 tracks could not be removed. Make sure the directory is writeable.", notDeletableTracks.count() ) );
-        KMessageBox::informationList(0,
-                                    text,
-                                    files,
-                                    i18n("Unable to be removed tracks") );
-    }
+    // start the remove workflow
+    prepareRemove( toRemove.toList() );
 }
 
 CollectionLocation*
diff --git a/src/collection/sqlcollection/SqlCollectionLocation.cpp b/src/collection/sqlcollection/SqlCollectionLocation.cpp
index d8bac2e..356cd13 100644
--- a/src/collection/sqlcollection/SqlCollectionLocation.cpp
+++ b/src/collection/sqlcollection/SqlCollectionLocation.cpp
@@ -111,19 +111,8 @@ SqlCollectionLocation::remove( const Meta::TrackPtr &track )
     if( sqlTrack && sqlTrack->inCollection() && sqlTrack->collection()->collectionId() == m_collection->collectionId() )
     {
         bool removed;
-        //SqlCollectionLocation uses KIO::move for moving files internally
-        //therefore we check whether the destination CollectionLocation
-        //represents the same collection, and check the existence of the
-        //file. If it does not exist, we assume that it has been moved, and remove it from the database.
-        //at worst we get a warning about a file not in the database. If it still exists, and
-        //this method has been called, do not do anything, as something is wrong.
-        //If the destination location is another collection, remove the file as we expect
-        //the destination to tell us if it is really really sure that it has copied a file.
-        //If we are not copying/moving files destination() will be 0.
-//         if( destination() && destination()->collection() == collection() )
-//         {
+        // we are going to delete it from the database only if is no longer on disk
         removed = !QFile::exists( sqlTrack->playableUrl().path() );
-//         }
         if( removed )
         {
 
@@ -159,6 +148,7 @@ SqlCollectionLocation::remove( const Meta::TrackPtr &track )
     }
     else
     {
+        debug() << "Remove Failed: track exists on disk." << sqlTrack->playableUrl().path();
         return false;
     }
 }
@@ -306,8 +296,14 @@ SqlCollectionLocation::slotRemoveJobFinished( KJob *job )
         warning() << "An error occurred when removing a file: " << job->errorString();
         transferError(m_removejobs.value( job ), KIO::buildErrorString( job->error(), job->errorString() ) );
     }
-    //we  assume that KIO works correctly...
-    transferSuccessful( m_removejobs.value( job ) );
+    else
+    {
+        // Remove the track from the database
+        remove( m_removejobs.value( job ) );
+
+        //we  assume that KIO works correctly...
+        transferSuccessful( m_removejobs.value( job ) );
+    }
 
     m_removejobs.remove( job );
     job->deleteLater();
diff --git a/src/collection/sqlcollection/SqlCollectionLocation.h b/src/collection/sqlcollection/SqlCollectionLocation.h
index 1f9bff3..0533708 100644
--- a/src/collection/sqlcollection/SqlCollectionLocation.h
+++ b/src/collection/sqlcollection/SqlCollectionLocation.h
@@ -38,6 +38,13 @@ class SqlCollectionLocation : public CollectionLocation
         virtual QStringList actualLocation() const;
         virtual bool isWritable() const;
         virtual bool isOrganizable() const;
+
+        /**
+         * Removes a track from the database ONLY if the file does NOT exist on disk.
+         * Do not call this method directly. Use the prepareRemove() method.
+         * @param track a track that does not exist on disk to be removed from the database
+         * @return true if the database entry was removed
+         */
         virtual bool remove( const Meta::TrackPtr &track );
         virtual void insertTracks( const QMap<Meta::TrackPtr, QString> &trackMap );
         virtual void insertStatistics( const QMap<Meta::TrackPtr, QString> &trackMap );