Index: src/metabundle.h =================================================================== --- src/metabundle.h (revision 576293) +++ src/metabundle.h (working copy) @@ -256,6 +256,8 @@ public: //accessors QString prettyFilesize() const; QString prettyRating() const; + bool safeToSave() { return m_safeToSave; } + public: //modifiers void setUrl( const KURL &url ); void setPath( const QString &path ); @@ -286,11 +288,11 @@ public: //modifiers void setLastFmBundle( const LastFm::Bundle &last ); void setUniqueId(); //will find the fileref void setUniqueId( const QString &id ); //WARNING WARNING WARNING SEE COMMENT in .CPP - bool setUniqueId( TagLib::FileRef &fileref, bool recreate, bool strip = false ); + bool readUniqueId(); + bool createUniqueId( TagLib::FileRef* fileref, bool stripOnly = false ); bool removeUniqueId(); bool newUniqueId(); void scannerAcknowledged(); - bool safeToSave() { return m_safeToSave; } void detach(); // for being able to apply QDeepCopy<> @@ -360,6 +362,10 @@ protected: bool m_notCompilation: 1; bool m_safeToSave: 1; int m_waitingOnKIO; + QString m_tempSavePath; + QString m_origRenamedSavePath; + QCString m_tempSaveDigest; + TagLib::FileRef* m_saveFileref; PodcastEpisodeBundle *m_podcastBundle; LastFm::Bundle *m_lastFmBundle; @@ -384,7 +390,9 @@ private: TagLib::ID3v2::UniqueFileIdentifierFrame *ourMP3UidFrame( TagLib::MPEG::File *file, QString ourId ); bool scannerSafeSave( TagLib::File* file ); - bool doSave( TagLib::File* file ); + TagLib::FileRef* prepareToSave(); + bool doSave( TagLib::FileRef* fileref ); + bool cleanupSave(); void abortSave( const QString message ); }; Index: src/metabundle.cpp =================================================================== --- src/metabundle.cpp (revision 576293) +++ src/metabundle.cpp (working copy) @@ -182,6 +182,10 @@ MetaBundle::MetaBundle() , m_notCompilation( false ) , m_safeToSave( false ) , m_waitingOnKIO( 0 ) + , m_tempSavePath( QString::null ) + , m_origRenamedSavePath( QString::null ) + , m_tempSaveDigest( 0 ) + , m_saveFileref( 0 ) , m_podcastBundle( 0 ) , m_lastFmBundle( 0 ) { @@ -211,6 +215,10 @@ MetaBundle::MetaBundle( const KURL &url, , m_notCompilation( false ) , m_safeToSave( false ) , m_waitingOnKIO( 0 ) + , m_tempSavePath( QString::null ) + , m_origRenamedSavePath( QString::null ) + , m_tempSaveDigest( 0 ) + , m_saveFileref( 0 ) , m_podcastBundle( 0 ) , m_lastFmBundle( 0 ) { @@ -220,7 +228,20 @@ MetaBundle::MetaBundle( const KURL &url, m_isValidMedia = CollectionDB::instance()->bundleForUrl( this ); if ( !isValidMedia() || ( !m_podcastBundle && m_length <= 0 ) ) + { readTags( readStyle, images ); + bool readOkay = readUniqueId(); //if read failed, probably don't want to try setting + if( m_uniqueId == QString::null && + readOkay && isFile() && + AmarokConfig::advancedTagFeatures() && + QString( kapp->name() ) != QString( "amarokcollectionscanner" ) ) + { + TagLib::FileRef *fileref = prepareToSave(); + if( fileref && createUniqueId( fileref ) ) + doSave( fileref ); + cleanupSave(); + } + } } else { @@ -262,6 +283,10 @@ MetaBundle::MetaBundle( const QString& t , m_notCompilation( false ) , m_safeToSave( false ) , m_waitingOnKIO( 0 ) + , m_tempSavePath( QString::null ) + , m_origRenamedSavePath( QString::null ) + , m_tempSaveDigest( 0 ) + , m_saveFileref( 0 ) , m_podcastBundle( 0 ) , m_lastFmBundle( 0 ) { @@ -321,6 +346,10 @@ MetaBundle::operator=( const MetaBundle& m_notCompilation = bundle.m_notCompilation; m_safeToSave = bundle.m_safeToSave; m_waitingOnKIO = bundle.m_waitingOnKIO; + m_tempSavePath = bundle.m_tempSavePath; + m_origRenamedSavePath = bundle.m_origRenamedSavePath; + m_tempSaveDigest = bundle.m_tempSaveDigest; + m_saveFileref = bundle.m_saveFileref; // delete m_podcastBundle; why does this crash Amarok? apparently m_podcastBundle isn't always initialized. m_podcastBundle = 0; @@ -566,8 +595,6 @@ MetaBundle::readTags( TagLib::AudioPrope } } - setUniqueId( fileref, false ); - if ( !disc.isEmpty() ) { int i = disc.find ('/'); @@ -1291,15 +1318,15 @@ MetaBundle::save() if( !isFile() ) return false; + bool problem; + //Set default codec to UTF-8 (see bugs 111246 and 111232) TagLib::ID3v2::FrameFactory::instance()->setDefaultTextEncoding(TagLib::String::UTF8); + TagLib::FileRef* f = prepareToSave(); - QCString path = QFile::encodeName( url().path() ); - TagLib::FileRef f( path, false ); - - if ( !f.isNull() ) + if ( f ) { - TagLib::Tag * t = f.tag(); + TagLib::Tag * t = f->tag(); if ( t ) { // f.tag() can return null if the file couldn't be opened for writing t->setTitle( QStringToTString( title() ) ); t->setArtist( QStringToTString( artist().string() ) ); @@ -1311,16 +1338,17 @@ MetaBundle::save() if ( hasExtendedMetaInformation() ) { - setExtendedTag( f.file(), composerTag, composer() ); - setExtendedTag( f.file(), discNumberTag, discNumber() ? QString::number( discNumber() ) : QString() ); - setExtendedTag( f.file(), bpmTag, bpm() ? QString::number( bpm() ) : QString() ); + setExtendedTag( f->file(), composerTag, composer() ); + setExtendedTag( f->file(), discNumberTag, discNumber() ? QString::number( discNumber() ) : QString() ); + setExtendedTag( f->file(), bpmTag, bpm() ? QString::number( bpm() ) : QString() ); if ( compilation() != CompilationUnknown ) - setExtendedTag( f.file(), compilationTag, QString::number( compilation() ) ); + setExtendedTag( f->file(), compilationTag, QString::number( compilation() ) ); } - return scannerSafeSave( f.file() ); + problem = doSave( f ); } } - return false; + + return !problem && cleanupSave(); } bool MetaBundle::save( QTextStream &stream, const QStringList &attributes, int indent ) const @@ -1372,7 +1400,7 @@ void MetaBundle::setUniqueId() { //if the file isn't already in the database, not checking for amarokcollectionscanner //will result in the UID being set to QString::null during the scan...bad! - if( !isFile() || ( QString( kapp->name() ) == QString( "amarokcollectionscanner" ) ) ) + if( !isFile() ) return; m_uniqueId = CollectionDB::instance()->uniqueIdFromUrl( url() ); @@ -1407,170 +1435,181 @@ MetaBundle::ourMP3UidFrame( TagLib::MPEG } bool -MetaBundle::setUniqueId( TagLib::FileRef &fileref, bool recreate, bool strip ) +MetaBundle::readUniqueId() { - if( !isFile() || ( ( QString( kapp->name() ) != QString( "amarokcollectionscanner" ) ) && !recreate && !strip ) ) + if( !isFile() ) return false; - //debug() << "recreate = " << (recreate?"true":"false") << endl; + QString ourId = QString( "Amarok - rediscover your music at http://amarok.kde.org" ).upper(); - bool createID = false; - int randSize = 8; - bool newID = false; + const QString path = url().path(); + TagLib::FileRef fileref; + fileref = TagLib::FileRef( QFile::encodeName( path ), true, TagLib::AudioProperties::Fast ); - QString ourId = QString( "Amarok - rediscover your music at http://amarok.kde.org" ).upper(); + if( fileref.isNull() ) + return false; - //due to bug in taglib (see bugs 132019 and 132018) it seems the genre isn't correctly updated from - //a preexisting id3v2.3 tag...so explicitly set it if ( TagLib::MPEG::File *file = dynamic_cast( fileref.file() ) ) { - if ( file->ID3v2Tag( AmarokConfig::advancedTagFeatures() ) ) + if ( file->ID3v2Tag( false ) ) { - if( file->ID3v2Tag()->frameListMap()["UFID"].isEmpty() || !ourMP3UidFrame( file, ourId ) || recreate || strip ) - createID = true; + if( file->ID3v2Tag()->frameListMap()["UFID"].isEmpty() || !ourMP3UidFrame( file, ourId ) ) + m_uniqueId = QString::null; else - { m_uniqueId = TStringToQString( TagLib::String( ourMP3UidFrame( file, ourId )->identifier() ) ); + + return true; + } } - if( ( strip || ( AmarokConfig::advancedTagFeatures() && createID ) ) - && TagLib::File::isWritable( file->name() ) ) + else if ( TagLib::Ogg::Vorbis::File *file = dynamic_cast( fileref.file() ) ) + { + if( file->tag() ) + { + if( file->tag()->fieldListMap().contains( QStringToTString( ourId ) ) && + !file->tag()->fieldListMap()[QStringToTString( ourId )].isEmpty() ) + m_uniqueId = TStringToQString( file->tag()->fieldListMap()[QStringToTString( ourId )].front() ); + else + m_uniqueId = QString::null; + + return true; + } + } + else if ( TagLib::FLAC::File *file = dynamic_cast( fileref.file() ) ) + { + if( file->xiphComment( false ) ) + { + if( file->xiphComment()->fieldListMap().contains( QStringToTString( ourId ) ) && + !file->xiphComment()->fieldListMap()[QStringToTString( ourId )].isEmpty() ) + m_uniqueId = TStringToQString( file->xiphComment()->fieldListMap()[QStringToTString( ourId )].front() ); + else + m_uniqueId = QString::null; + + return true; + } + } + else if ( TagLib::Ogg::FLAC::File *file = dynamic_cast( fileref.file() ) ) + { + if( file->tag() ) + { + if( file->tag()->fieldListMap().contains( QStringToTString( ourId ) ) && + !file->tag()->fieldListMap()[QStringToTString( ourId )].isEmpty() ) + m_uniqueId = TStringToQString( file->tag()->fieldListMap()[QStringToTString( ourId )].front() ); + else + m_uniqueId = QString::null; + + return true; + } + } + else if ( TagLib::MP4::File *file = dynamic_cast( fileref.file() ) ) + { + if( file || !file ) + return false; //not handled, at least not yet + } + return false; +} + +bool +MetaBundle::createUniqueId( TagLib::FileRef *fileref, bool stripOnly ) +{ + if( !isFile() || !TagLib::File::isWritable( fileref->file()->name() ) ) + return false; + + int randSize = 8; + + QString ourId = QString( "Amarok - rediscover your music at http://amarok.kde.org" ).upper(); + + //due to bug in taglib (see bugs 132019 and 132018) it seems the genre isn't correctly updated from + //a preexisting id3v2.3 tag...so explicitly set it + if ( TagLib::MPEG::File *file = dynamic_cast( fileref->file() ) ) + { + if ( file->ID3v2Tag( true ) ) { - m_uniqueId = getRandomStringHelper( randSize ); if( !file->ID3v2Tag()->frameListMap()["UFID"].isEmpty() && ourMP3UidFrame( file, ourId ) ) file->ID3v2Tag()->removeFrame( ourMP3UidFrame( file, ourId ) ); - if( strip ) + if( stripOnly ) { file->ID3v2Tag()->setGenre( file->ID3v2Tag()->genre() ); - scannerSafeSave( file ); + m_uniqueId = QString::null; return true; } + m_uniqueId = getRandomStringHelper( randSize ); file->ID3v2Tag()->addFrame( new TagLib::ID3v2::UniqueFileIdentifierFrame( QStringToTString( ourId ), TagLib::ByteVector( m_uniqueId.ascii(), randSize ) ) ); file->ID3v2Tag()->setGenre( file->ID3v2Tag()->genre() ); - scannerSafeSave( file ); - newID = true; - } + return true; } } - else if ( TagLib::Ogg::Vorbis::File *file = dynamic_cast( fileref.file() ) ) + else if ( TagLib::Ogg::Vorbis::File *file = dynamic_cast( fileref->file() ) ) { if( file->tag() ) { - //For now remove all tag-writing options for Vorbis files due to seeming Ogg corruption - //for some people...and keep bugging wheels - - if( ( file->tag()->fieldListMap().contains( QStringToTString( ourId ) ) && + if( file->tag()->fieldListMap().contains( QStringToTString( ourId ) ) && !file->tag()->fieldListMap()[QStringToTString( ourId )].isEmpty() ) - && ( recreate || strip ) - && AmarokConfig::advancedTagFeatures() - && TagLib::File::isWritable( file->name() ) ) - { file->tag()->removeField( QStringToTString( ourId ) ); - } - if( AmarokConfig::advancedTagFeatures() && strip ) + if( stripOnly ) { - scannerSafeSave( file ); + m_uniqueId = QString::null; return true; } - if( !file->tag()->fieldListMap().contains( QStringToTString( ourId ) ) || - ( file->tag()->fieldListMap().contains( QStringToTString( ourId ) ) && - !file->tag()->fieldListMap()[QStringToTString( ourId )].isEmpty() ) - || recreate ) - { - if( AmarokConfig::advancedTagFeatures() && TagLib::File::isWritable( file->name() ) ) - { m_uniqueId = getRandomStringHelper( randSize ); file->tag()->addField( QStringToTString( ourId ), QStringToTString( m_uniqueId ) ); - scannerSafeSave( file ); - newID = true; - } - } - else - { - m_uniqueId = TStringToQString( file->tag()->fieldListMap()[QStringToTString( ourId )].front() ); - } + return true; } } //FLACs are commented because supposedly they have to be totally rewritten to add uniqueids, and as they're big this takes a while. //This will have to be tested. It's also supposedly a TagLib bug so maybe it could be fixed, especially with some pressure on wheels - else if ( TagLib::FLAC::File *file = dynamic_cast( fileref.file() ) ) - { - if ( file->xiphComment( AmarokConfig::advancedTagFeatures() ) ) + else if ( TagLib::FLAC::File *file = dynamic_cast( fileref->file() ) ) { - /* - if( file->xiphComment()->fieldListMap().contains( QStringToTString( ourId ) ) - && AmarokConfig::advancedTagFeatures() - && ( recreate || strip ) - && TagLib::File::isWritable( file->name() ) ) + if( file->xiphComment( true ) ) + {/* + if( file->xiphComment()->fieldListMap().contains( QStringToTString( ourId ) ) && + !file->xiphComment()->fieldListMap()[QStringToTString( ourId )].isEmpty() ) file->xiphComment()->removeField( QStringToTString( ourId ) ); - if( AmarokConfig::advancedTagFeatures() && strip ) + if( stripOnly ) { - scannerSafeSave( file ); + m_uniqueId = QString::null; return true; } - */ - if( !file->xiphComment()->fieldListMap().contains( QStringToTString( ourId ) ) ) - { - /* - if( AmarokConfig::advancedTagFeatures() && TagLib::File::isWritable( file->name() ) ) - { + m_uniqueId = getRandomStringHelper( randSize ); file->xiphComment()->addField( QStringToTString( ourId ), QStringToTString( m_uniqueId ) ); - scannerSafeSave( file ); - newID = true; - } - */ - } - else - m_uniqueId = TStringToQString( file->xiphComment()->fieldListMap()[QStringToTString( ourId )].front() ); - } + return true; + */} } - else if ( TagLib::Ogg::FLAC::File *file = dynamic_cast( fileref.file() ) ) + else if ( TagLib::Ogg::FLAC::File *file = dynamic_cast( fileref->file() ) ) { if( file->tag() ) - { - /* - if( file->tag()->fieldListMap().contains( QStringToTString( ourId ) ) - && AmarokConfig::advancedTagFeatures() - && ( recreate || strip ) - && TagLib::File::isWritable( file->name() ) ) + {/* + if( file->tag()->fieldListMap().contains( QStringToTString( ourId ) ) && + !file->tag()->fieldListMap()[QStringToTString( ourId )].isEmpty() ) file->tag()->removeField( QStringToTString( ourId ) ); - if( AmarokConfig::advancedTagFeatures() && strip ) + if( stripOnly ) { - scannerSafeSave( file ); + m_uniqueId = QString::null; return true; } - */ - if( !file->tag()->fieldListMap().contains( QStringToTString( ourId ) ) ) - { - /* - if( AmarokConfig::advancedTagFeatures() && TagLib::File::isWritable( file->name() ) ) - { + m_uniqueId = getRandomStringHelper( randSize ); - file->tag()->addField( QStringToTString( ourId ), QStringToTString( m_uniqueId ) ), - scannerSafeSave( file ); - newID = true; - } - */ - } - else - m_uniqueId = TStringToQString( file->tag()->fieldListMap()[QStringToTString( ourId )].front() ); - } + file->tag()->addField( QStringToTString( ourId ), QStringToTString( m_uniqueId ) ); + return true; + */} + } - else if ( TagLib::MP4::File *file = dynamic_cast( fileref.file() ) ) + else if ( TagLib::MP4::File *file = dynamic_cast( fileref->file() ) ) { if( file || !file ) - return false; //not handled, at least not yet + return false; } - return (recreate ? recreate && newID : !m_uniqueId.isEmpty() ); + + return false; } bool @@ -1641,36 +1680,27 @@ MetaBundle::scannerSafeSave( TagLib::Fil return result; } -bool -MetaBundle::doSave( TagLib::File* /*file*/ ) +TagLib::FileRef * +MetaBundle::prepareToSave() { - - //TODO: much commenting needed. For now this pretty much follows algorithm laid out in bug 131353, - //but isn't useable since I need to find a good way to switch the file path with taglib, or a good way - //to get all the metadata copied over. - DEBUG_BLOCK - bool revert = false; const KURL origPath = url(); char buf[32]; int hostname = gethostname(buf, 32); if( hostname != 0 ) { abortSave( "Could not determine hostname!" ); - return false; + return 0; } QString pid; - const QString tempPath = origPath.path() + ".amaroktemp.host-" + QString( buf ) + ".pid-" + pid.setNum( getpid() ); - const QString origRenamedPath = origPath.path() + ".original.amaroktemp.host-" + QString( buf ) + ".pid-" + pid.setNum( getpid() ); + m_tempSavePath = origPath.path() + ".amaroktemp.host-" + QString( buf ) + ".pid-" + pid.setNum( getpid() ); + m_origRenamedSavePath = origPath.path() + ".original.amaroktemp.host-" + QString( buf ) + ".pid-" + pid.setNum( getpid() ); - KIO::SimpleJob *deletejob, *deletejob2; KIO::FileCopyJob *copyjob; - KIO::FileCopyJob *movejob, *movejob2, *movejob3; - - QFile *tempFile, *origRenamedFile; + QFile *tempFile; QCString tempDigest, origRenamedDigest; - copyjob = KIO::file_copy( url(), KURL::fromPathOrURL( tempPath ), -1, false, false, false ); + copyjob = KIO::file_copy( url(), KURL::fromPathOrURL( m_tempSavePath ), -1, false, false, false ); connect( copyjob, SIGNAL( result(KIO::Job*) ), SLOT( kioDone(KIO::Job*) ) ); m_waitingOnKIO = 1; @@ -1680,21 +1710,58 @@ MetaBundle::doSave( TagLib::File* /*file if( m_waitingOnKIO != 0 ) { abortSave( "Could not create copy!" ); - return false; + return 0; } - tempFile = new QFile( tempPath ); + //By this point, we have the following: + //The original file is copied at path m_tempSavePath + //We have generated what will be the filename to rename the original to in m_origRenamedSavePath + //We have successfully copied the original file to the temp location + + tempFile = new QFile( m_tempSavePath ); tempFile->open( IO_ReadOnly ); - KMD5 md5sum( tempFile->readAll() ); - tempDigest = md5sum.hexDigest(); + KMD5* md5sum = new KMD5( tempFile->readAll() ); + m_tempSaveDigest = md5sum->hexDigest(); + + debug() << "MD5 sum of temp file: " << m_tempSaveDigest.data() << endl; + + //Now, we have a MD5 sum of the original file at the time of copying saved in m_tempSaveDigest + //Create a fileref on the copied file, for modification - debug() << "MD5 sum of temp file: " << tempDigest.data() << endl; + m_saveFileref = new TagLib::FileRef( QFile::encodeName( m_tempSavePath ), false ); - //NOTE: check correctness of file somehow? what if some other process modifying during our copy? - //could be a "not our problem" but is there a better way? - //TODO: modify tags in temporary file -- new TagLib fileref to new file, copy over metadata? + if( !m_saveFileref->isNull() ) + return m_saveFileref; - movejob = KIO::file_move( url(), KURL::fromPathOrURL( origRenamedPath ), -1, false, false, false ); + abortSave( "Error creating temp file's fileref!" ); + return 0; +} + +bool +MetaBundle::doSave( TagLib::FileRef* fileref ) +{ + //TODO: much commenting needed. For now this pretty much follows algorithm laid out in bug 131353, + //but isn't useable since I need to find a good way to switch the file path with taglib, or a good way + //to get all the metadata copied over. + + DEBUG_BLOCK + bool revert = false; + + KIO::SimpleJob *deletejob, *deletejob2; + KIO::FileCopyJob *movejob, *movejob2, *movejob3; + QFile* origRenamedFile; + KMD5* md5sum; + + QCString origRenamedDigest; + + //We've made our changes to the fileref; save it first, then do the logic to move the correct file back + if( !fileref->save() ) + { + abortSave( "Could not save the new file!" ); + goto fail_remove_copy; + } + + movejob = KIO::file_move( url(), KURL::fromPathOrURL( m_origRenamedSavePath ), -1, false, false, false ); connect( movejob, SIGNAL( result(KIO::Job*) ), SLOT( kioDone(KIO::Job*) ) ); m_waitingOnKIO = 1; @@ -1707,22 +1774,21 @@ MetaBundle::doSave( TagLib::File* /*file goto fail_remove_copy; } - md5sum.reset(); - origRenamedFile = new QFile( origRenamedPath ); + origRenamedFile = new QFile( m_origRenamedSavePath ); origRenamedFile->open( IO_ReadOnly ); - md5sum.update( origRenamedFile->readAll() ); - origRenamedDigest = md5sum.hexDigest(); + md5sum = new KMD5( origRenamedFile->readAll() ); + origRenamedDigest = md5sum->hexDigest(); debug() << "md5sum of original file: " << origRenamedDigest.data() << endl; - if( origRenamedDigest != tempDigest ) + if( origRenamedDigest != m_tempSaveDigest ) { abortSave( "Original checksum did not match current checksum!" ); revert = true; goto fail_remove_copy; } - movejob2 = KIO::file_move( KURL::fromPathOrURL( tempPath ), url(), -1, false, false, false ); + movejob2 = KIO::file_move( KURL::fromPathOrURL( m_tempSavePath ), url(), -1, false, false, false ); connect( movejob2, SIGNAL( result(KIO::Job*) ), SLOT( kioDone(KIO::Job*) ) ); m_waitingOnKIO = 1; @@ -1736,7 +1802,7 @@ MetaBundle::doSave( TagLib::File* /*file goto fail_remove_copy; } - deletejob = KIO::file_delete( KURL::fromPathOrURL( origRenamedPath ), false ); + deletejob = KIO::file_delete( KURL::fromPathOrURL( m_origRenamedSavePath ), false ); connect( deletejob, SIGNAL( result(KIO::Job*) ), SLOT( kioDone(KIO::Job*) ) ); m_waitingOnKIO = 1; @@ -1746,14 +1812,14 @@ MetaBundle::doSave( TagLib::File* /*file if( m_waitingOnKIO != 0 ) { abortSave( "Could not delete the original file!" ); - amaroK::StatusBar::instance()->longMessageThreadSafe( QString( "Could not remove the copy of the original file, located at %1. Please remove it manually." ).arg( origRenamedPath ) ); + amaroK::StatusBar::instance()->longMessageThreadSafe( QString( "Could not remove the copy of the original file, located at %1. Please remove it manually." ).arg( m_origRenamedSavePath ) ); return false; } return true; fail_remove_copy: - deletejob2 = KIO::file_delete( KURL::fromPathOrURL( tempPath ), false ); + deletejob2 = KIO::file_delete( KURL::fromPathOrURL( m_tempSavePath ), false ); connect( deletejob2, SIGNAL( result(KIO::Job*) ), SLOT( kioDone(KIO::Job*) ) ); m_waitingOnKIO = 1; @@ -1763,13 +1829,13 @@ MetaBundle::doSave( TagLib::File* /*file if( m_waitingOnKIO != 0 ) { abortSave( "Could not delete the temporary file!" ); - amaroK::StatusBar::instance()->longMessageThreadSafe( QString( "Could not remove the temporary file %1. Please remove it manually." ).arg( tempPath ) ); + amaroK::StatusBar::instance()->longMessageThreadSafe( QString( "Could not remove the temporary file %1. Please remove it manually." ).arg( m_tempSavePath ) ); } if( !revert ) return false; - movejob3 = KIO::file_move( KURL::fromPathOrURL( origRenamedPath ), url(), -1, false, false, false ); + movejob3 = KIO::file_move( KURL::fromPathOrURL( m_origRenamedSavePath ), url(), -1, false, false, false ); connect( movejob3, SIGNAL( result(KIO::Job*) ), SLOT( kioDone(KIO::Job*) ) ); m_waitingOnKIO = 1; @@ -1779,12 +1845,34 @@ MetaBundle::doSave( TagLib::File* /*file if( m_waitingOnKIO != 0 ) { abortSave( "Could not revert file to original filename!" ); - amaroK::StatusBar::instance()->longMessageThreadSafe( QString( "Could not revert the file to its original filename, from %1 to %2. Please do this manually." ).arg( origRenamedPath ).arg( origPath.path() ) ); + amaroK::StatusBar::instance()->longMessageThreadSafe( QString( "Could not revert the file to its original filename, from %1 to %2. Please do this manually." ).arg( m_origRenamedSavePath ).arg( url().path() ) ); } return false; } +bool +MetaBundle::cleanupSave() +{ + DEBUG_BLOCK + + bool dirty = false; + + if( QFile::exists( m_tempSavePath ) && !QFile::remove( m_tempSavePath ) ) + { + dirty = true; + abortSave( "Could not delete the temporary file!" ); + amaroK::StatusBar::instance()->longMessageThreadSafe( QString( "Could not remove the temporary file %1. Please remove it manually." ).arg( m_tempSavePath ) ); + } + + m_tempSavePath == QString::null; + m_origRenamedSavePath = QString::null; + m_tempSaveDigest = QCString( 0 ); + delete m_saveFileref; + + return !dirty; +} + void MetaBundle::kioDone( KIO::Job *job ) { @@ -1819,17 +1907,18 @@ MetaBundle::newUniqueId() if( !isFile() || !AmarokConfig::advancedTagFeatures() ) return false; - const QString path = url().path(); - TagLib::FileRef fileref; - fileref = TagLib::FileRef( QFile::encodeName( path ), true, TagLib::AudioProperties::Fast ); + TagLib::FileRef *fileref = prepareToSave(); - if( !fileref.isNull() ) - return setUniqueId( fileref, true ); - else + if( fileref ) { debug() << "ERROR: failed to set new uniqueid (could not open fileref)" << endl; return false; } + + if( createUniqueId( fileref ) ) + return ( doSave( fileref ) && cleanupSave() ); + + return false; } bool @@ -1838,17 +1927,18 @@ MetaBundle::removeUniqueId() if( !isFile() ) return false; - const QString path = url().path(); - TagLib::FileRef fileref; - fileref = TagLib::FileRef( QFile::encodeName( path ), true, TagLib::AudioProperties::Fast ); + TagLib::FileRef *fileref = prepareToSave(); - if( !fileref.isNull() ) - return setUniqueId( fileref, false, true ); - else + if( fileref ) { debug() << "ERROR: failed to remove uniqueid (could not open fileref)" << endl; return false; } + + if( createUniqueId( fileref, true ) ) + return ( doSave( fileref ) && cleanupSave() ); + + return false; } int