Only instantiate Result objects with a valid Track

src/libtomahawk/Result.cpp

@@ -55,11 +55,10 @@ sourceCacheKey( Resolver* resolver, const QSize& size, TomahawkUtils::ImageMode
 
 
 Tomahawk::result_ptr
-Result::get( const QString& url )
+Result::get( const QString& url, const track_ptr& track )
 {
-    if ( url.trimmed().isEmpty() )
+    if ( url.trimmed().isEmpty() || track.isNull() )
     {
-//        Q_ASSERT( false );
         return result_ptr();
     }
 
@@ -69,22 +68,32 @@ Result::get( const QString& url )
         return s_results.value( url );
     }
 
-    result_ptr r = result_ptr( new Result( url ), &Result::deleteLater );
+    result_ptr r = result_ptr( new Result( url, track ), &Result::deleteLater );
     s_results.insert( url, r );
 
     return r;
 }
 
 
-bool
+result_ptr
-Result::isCached( const QString& url )
+Result::getCached( const QString& url )
 {
+    if ( url.trimmed().isEmpty() )
+    {
+        return result_ptr();
+    }
+
     QMutexLocker lock( &s_mutex );
-    return ( s_results.contains( url ) );
+    if ( s_results.contains( url ) )
+    {
+        return s_results.value( url );
+    }
+
+    return result_ptr();
 }
 
 
-Result::Result( const QString& url )
+Result::Result( const QString& url, const track_ptr& track )
     : QObject()
     , m_url( url )
     , m_checked( false )
@@ -93,6 +102,7 @@ Result::Result( const QString& url )
     , m_modtime( 0 )
     , m_score( 0 )
     , m_fileId( 0 )
+    , m_track( track )
 {
     connect( Pipeline::instance(), SIGNAL( resolverRemoved( Tomahawk::Resolver* ) ), SLOT( onResolverRemoved( Tomahawk::Resolver* ) ), Qt::QueuedConnection );
 }
@@ -265,7 +275,7 @@ Result::toQuery()
         m_query = query->weakRef();
 
         QList<Tomahawk::result_ptr> rl;
-        rl << Result::get( m_url );
+        rl << Result::get( m_url, m_track );
 
         query->addResults( rl );
         query->setResolveFinished( true );

src/libtomahawk/Result.h

@@ -52,8 +52,24 @@ friend class DatabaseCommand_AddFiles;
 friend class DatabaseCommand_LoadFile;
 
 public:
-    static Tomahawk::result_ptr get( const QString& url );
+    /**
-    static bool isCached( const QString& url );
+     * Get a Result instance for an URL if it is cached, otherwise create a new
+     * instance using the supplied Track object.
+     */
+    static Tomahawk::result_ptr get( const QString& url,
+                                     const Tomahawk::track_ptr& track );
+
+    /**
+     * Get a Result instance for an URL if it is already cached.
+     *
+     * This will not create a new Result instance if there is no matching
+     * Result in the cache, use Result::get for this.
+     *
+     * @param url Unique result identifier
+     * @return nullptr if the Result is not yet cached
+     */
+    static Tomahawk::result_ptr getCached( const QString& url );
+
     virtual ~Result();
 
     bool isValid() const;
@@ -130,7 +146,7 @@ private slots:
 
 private:
     // private constructor
-    explicit Result( const QString& url );
+    explicit Result( const QString& url, const Tomahawk::track_ptr& track );
     explicit Result();
 
     mutable RID m_rid;

src/libtomahawk/audio/AudioEngine.cpp

@@ -936,7 +936,6 @@ AudioEngine::play( const QUrl& url )
 {
     tDebug() << Q_FUNC_INFO << url;
 
-    result_ptr result = Result::get( url.toString() );
     const QVariantMap tags = MusicScanner::readTags( QFileInfo( url.toLocalFile() ) ).toMap();
 
     track_ptr t;
@@ -945,19 +944,22 @@ AudioEngine::play( const QUrl& url )
         t = Track::get( tags["artist"].toString(), tags["track"].toString(), tags["album"].toString(),
                         tags["duration"].toInt(), tags["composer"].toString(),
                         tags["albumpos"].toUInt(), tags["discnumber"].toUInt() );
-
-        result->setSize( tags["size"].toUInt() );
-        result->setBitrate( tags["bitrate"].toUInt() );
-        result->setModificationTime( tags["mtime"].toUInt() );
-        result->setMimetype( tags["mimetype"].toString() );
     }
     else
     {
         t = Tomahawk::Track::get( "Unknown Artist", "Unknown Track" );
     }
 
+    result_ptr result = Result::get( url.toString(), t );
+
+    if ( !tags.isEmpty() )
+    {
+        result->setSize( tags["size"].toUInt() );
+        result->setBitrate( tags["bitrate"].toUInt() );
+        result->setModificationTime( tags["mtime"].toUInt() );
+        result->setMimetype( tags["mimetype"].toString() );
+    }
 
-    result->setTrack( t );
     result->setScore( 1.0 );
     result->setCollection( SourceList::instance()->getLocal()->collections().first(), false );
 

src/libtomahawk/database/DatabaseCommand_AllTracks.cpp

@@ -138,16 +138,16 @@ DatabaseCommand_AllTracks::exec( DatabaseImpl* dbi )
         if ( !s->isLocal() )
             url = QString( "servent://%1\t%2" ).arg( s->nodeId() ).arg( url );
 
-        Tomahawk::result_ptr result = Tomahawk::Result::get( url );
         Tomahawk::track_ptr t = Tomahawk::Track::get( trackId,
                                                       artist, track, album,
                                                       duration, composer,
                                                       albumpos, discnumber );
+
         if ( m_album || m_artist ) {
             t->loadAttributes();
         }
-        result->setTrack( t );
 
+        Tomahawk::result_ptr result = Tomahawk::Result::get( url, t );
         result->setSize( size );
         result->setBitrate( bitrate );
         result->setModificationTime( modificationTime );

src/libtomahawk/database/DatabaseCommand_Resolve.cpp

@@ -136,8 +136,8 @@ DatabaseCommand_Resolve::resolve( DatabaseImpl* lib )
         if ( !s->isLocal() )
             url = QString( "servent://%1\t%2" ).arg( s->nodeId() ).arg( url );
 
-        Tomahawk::result_ptr result = Tomahawk::Result::get( url );
+        Tomahawk::result_ptr result = Tomahawk::Result::getCached( url );
-        if ( result->isValid() )
+        if ( result )
         {
             tDebug( LOGVERBOSE ) << "Result already cached:" << result->toString();
             res << result;
@@ -146,8 +146,8 @@ DatabaseCommand_Resolve::resolve( DatabaseImpl* lib )
 
         track_ptr track = Track::get( files_query.value( 9 ).toUInt(), files_query.value( 12 ).toString(), files_query.value( 14 ).toString(), files_query.value( 13 ).toString(), files_query.value( 5 ).toUInt(), files_query.value( 15 ).toString(), files_query.value( 17 ).toUInt(), files_query.value( 11 ).toUInt() );
         track->loadAttributes();
-        result->setTrack( track );
 
+        result = Result::get( url, track );
         result->setModificationTime( files_query.value( 1 ).toUInt() );
         result->setSize( files_query.value( 2 ).toUInt() );
         result->setMimetype( files_query.value( 4 ).toString() );
@@ -244,19 +244,18 @@ DatabaseCommand_Resolve::fullTextResolve( DatabaseImpl* lib )
         if ( !s->isLocal() )
             url = QString( "servent://%1\t%2" ).arg( s->nodeId() ).arg( url );
 
-        bool cached = Tomahawk::Result::isCached( url );
+        Tomahawk::result_ptr result = Tomahawk::Result::getCached( url );
-        Tomahawk::result_ptr result = Tomahawk::Result::get( url );
+        if ( result )
-        if ( cached )
         {
-            qDebug() << "Result already cached:" << result->toString();
+            tDebug( LOGVERBOSE ) << "Result already cached:" << result->toString();
             res << result;
             continue;
         }
 
         track_ptr track = Track::get( files_query.value( 9 ).toUInt(), files_query.value( 12 ).toString(), files_query.value( 14 ).toString(), files_query.value( 13 ).toString(), files_query.value( 5 ).toUInt(), files_query.value( 15 ).toString(), files_query.value( 17 ).toUInt(), files_query.value( 11 ).toUInt() );
         track->loadAttributes();
-        result->setTrack( track );
 
+        result = Result::get( url, track );
         result->setModificationTime( files_query.value( 1 ).toUInt() );
         result->setSize( files_query.value( 2 ).toUInt() );
         result->setMimetype( files_query.value( 4 ).toString() );

src/libtomahawk/database/DatabaseImpl.cpp

@@ -307,11 +307,9 @@ Tomahawk::DatabaseImpl::file( int fid )
         if ( !s->isLocal() )
             url = QString( "servent://%1\t%2" ).arg( s->nodeId() ).arg( url );
 
-        r = Tomahawk::Result::get( url );
-
         Tomahawk::track_ptr track = Tomahawk::Track::get( query.value( 9 ).toUInt(), query.value( 11 ).toString(), query.value( 13 ).toString(), query.value( 12 ).toString(), query.value( 5 ).toUInt(), query.value( 14 ).toString(), 0, 0 );
-        r->setTrack( track );
 
+        r = Tomahawk::Result::get( url, track );
         r->setModificationTime( query.value( 1 ).toUInt() );
         r->setSize( query.value( 2 ).toUInt() );
         r->setMimetype( query.value( 4 ).toString() );
@@ -627,16 +625,18 @@ Tomahawk::DatabaseImpl::resultFromHint( const Tomahawk::query_ptr& origquery )
     }
     else if ( TomahawkUtils::whitelistedHttpResultHint( url ) )
     {
+        Tomahawk::track_ptr track = Tomahawk::Track::get( origquery->queryTrack()->artist(),
+                                                          origquery->queryTrack()->track(),
+                                                          origquery->queryTrack()->album(),
+                                                          origquery->queryTrack()->duration() );
+
         // Return http resulthint directly
-        res = Tomahawk::Result::get( url );
+        res = Tomahawk::Result::get( url, track );
         res->setRID( uuid() );
         res->setScore( 1.0 );
         const QUrl u = QUrl::fromUserInput( url );
         res->setFriendlySource( u.host() );
 
-        Tomahawk::track_ptr track = Tomahawk::Track::get( origquery->queryTrack()->artist(), origquery->queryTrack()->track(), origquery->queryTrack()->album(), origquery->queryTrack()->duration() );
-        res->setTrack( track );
-
         ResultUrlChecker* checker = new ResultUrlChecker( origquery, QList< result_ptr >() << res );
         QEventLoop loop;
         connect( checker, SIGNAL( done() ), &loop, SLOT( quit() ) );
@@ -694,12 +694,17 @@ Tomahawk::DatabaseImpl::resultFromHint( const Tomahawk::query_ptr& origquery )
         if ( !s->isLocal() )
             url = QString( "servent://%1\t%2" ).arg( s->nodeId() ).arg( url );
 
-        res = Tomahawk::Result::get( url );
+        Tomahawk::track_ptr track = Tomahawk::Track::get( query.value( 9 ).toUInt(),
-
+                                                          query.value( 11 ).toString(),
-        Tomahawk::track_ptr track = Tomahawk::Track::get( query.value( 9 ).toUInt(), query.value( 11 ).toString(), query.value( 13 ).toString(), query.value( 12 ).toString(), query.value( 5 ).toInt(), query.value( 14 ).toString(), query.value( 16 ).toUInt(), query.value( 17 ).toUInt() );
+                                                          query.value( 13 ).toString(),
+                                                          query.value( 12 ).toString(),
+                                                          query.value( 5 ).toInt(),
+                                                          query.value( 14 ).toString(),
+                                                          query.value( 16 ).toUInt(),
+                                                          query.value( 17 ).toUInt() );
         track->loadAttributes();
-        res->setTrack( track );
 
+        res = Tomahawk::Result::get( url, track );
         res->setModificationTime( query.value( 1 ).toUInt() );
         res->setSize( query.value( 2 ).toUInt() );
         res->setMimetype( query.value( 4 ).toString() );

src/libtomahawk/resolvers/JSResolver.cpp

@@ -563,10 +563,6 @@ JSResolver::parseResultVariantList( const QVariantList& reslist )
             duration = time.secsTo( QTime( 0, 0 ) ) * -1;
         }
 
-        Tomahawk::result_ptr rp = Tomahawk::Result::get( m.value( "url" ).toString() );
-        if ( !rp )
-            continue;
-
         Tomahawk::track_ptr track = Tomahawk::Track::get( m.value( "artist" ).toString(),
                                                           m.value( "track" ).toString(),
                                                           m.value( "album" ).toString(),
@@ -577,7 +573,10 @@ JSResolver::parseResultVariantList( const QVariantList& reslist )
         if ( !track )
             continue;
 
-        rp->setTrack( track );
+        Tomahawk::result_ptr rp = Tomahawk::Result::get( m.value( "url" ).toString(), track );
+        if ( !rp )
+            continue;
+
         rp->setBitrate( m.value( "bitrate" ).toUInt() );
         rp->setSize( m.value( "size" ).toUInt() );
         rp->setRID( uuid() );

src/libtomahawk/resolvers/ScriptResolver.cpp

@@ -289,15 +289,14 @@ ScriptResolver::handleMsg( const QByteArray& msg )
             QVariantMap m = rv.toMap();
             tDebug( LOGVERBOSE ) << "Found result:" << m;
 
-            Tomahawk::result_ptr rp = Tomahawk::Result::get( m.value( "url" ).toString() );
-            if ( !rp )
-                continue;
-
             Tomahawk::track_ptr track = Tomahawk::Track::get( m.value( "artist" ).toString(), m.value( "track" ).toString(), m.value( "album" ).toString(), m.value( "duration" ).toUInt(), QString(), m.value( "albumpos" ).toUInt(), m.value( "discnumber" ).toUInt() );
             if ( !track )
                 continue;
 
-            rp->setTrack( track );
+            Tomahawk::result_ptr rp = Tomahawk::Result::get( m.value( "url" ).toString(), track );
+            if ( !rp )
+                continue;
+
             rp->setBitrate( m.value( "bitrate" ).toUInt() );
             rp->setSize( m.value( "size" ).toUInt() );
             rp->setRID( uuid() );

src/tests/TestResult.h

@@ -28,23 +28,23 @@ class TestResult : public QObject
     Q_OBJECT
 
 private slots:
-    void testIsValid()
-    {
-        Tomahawk::result_ptr r = Tomahawk::Result::get( "/tmp/test.mp3" );
-        QVERIFY( !r->isValid() );
-
-        Tomahawk::track_ptr t = Tomahawk::Track::get( "Artist", "Track" );
-        r->setTrack( t );
-        QVERIFY( r->isValid() );
-    }
-
     void testGet()
     {
-        Tomahawk::result_ptr r = Tomahawk::Result::get( "" );
+        Tomahawk::result_ptr r;
+
+        r = Tomahawk::Result::get( "", Tomahawk::track_ptr() );
         QVERIFY( !r );
 
-        Tomahawk::result_ptr vr = Tomahawk::Result::get( "/tmp/test.mp3" );
+        r = Tomahawk::Result::get( "/tmp/test.mp3", Tomahawk::track_ptr() );
-        QVERIFY( vr );
+        QVERIFY( !r);
+
+        Tomahawk::track_ptr t = Tomahawk::Track::get( "Artist", "Track" );
+
+        r = Tomahawk::Result::get( "", t );
+        QVERIFY( !r);
+
+        r = Tomahawk::Result::get( "/tmp/test.mp3", t );
+        QVERIFY( r );
     }
 };