Refactor the chartsplugin parsing

src/libtomahawk/infosystem/infoplugins/generic/chartsplugin.cpp

@@ -1,6 +1,7 @@
 /* === This file is part of Tomahawk Player - <http://tomahawk-player.org> ===
  *
  *   Copyright 2010-2011, Hugo Lindström <hugolm84@gmail.com>
+ *   Copyright 2011, Leo Franchi <lfranchi@kde.org>
  *
  *   Tomahawk is free software: you can redistribute it and/or modify
  *   it under the terms of the GNU General Public License as published by
@@ -42,6 +43,7 @@ using namespace Tomahawk::InfoSystem;
 
 ChartsPlugin::ChartsPlugin()
     : InfoPlugin()
+    , m_chartsFetchJobs( 0 )
 {
 
 
@@ -72,14 +74,17 @@ ChartsPlugin::namChangedSlot( QNetworkAccessManager *nam )
     /// Then get each chart from resource
     /// We need to fetch them before they are asked for
 
-    if( !m_chartResources.isEmpty() && m_nam && m_chartTypes.isEmpty() ){
+    if( !m_chartResources.isEmpty() && m_nam ){
 
         tDebug() << "ChartsPlugin: InfoChart fetching possible resources";
-        foreach(QVariant resource, m_chartResources)
+        foreach ( QVariant resource, m_chartResources )
         {
             QUrl url = QUrl( QString( CHART_URL "source/%1" ).arg(resource.toString() ) );
             QNetworkReply* reply = m_nam.data()->get( QNetworkRequest( url ) );
+            tDebug() << "fetching:" << url;
             connect( reply, SIGNAL( finished() ), SLOT( chartTypes() ) );
+
+            m_chartsFetchJobs++;
         }
 
     }
@@ -222,13 +227,10 @@ ChartsPlugin::notInCacheSlot( uint requestId, QHash<QString, QString> criteria,
 
         case InfoChartCapabilities:
         {
-            if ( m_allChartsMap.isEmpty() )
+            if ( m_chartsFetchJobs > 0 )
             {
-
+                qDebug() << Q_FUNC_INFO << "InfoChartCapabilities still fetching!";
-                qDebug() << Q_FUNC_INFO << "InfoChartCapabilities is empty, probably still fetching!";
+                m_cachedRequests.append( QPair< uint, InfoRequestData >( requestId, requestData ) );
-                m_cachedRequests.append( QPair< uint, InfoRequestData >( requestId, requestData ) );;
-
-//                 dataError( requestId, requestData );
                 return;
             }
 
@@ -253,15 +255,16 @@ ChartsPlugin::notInCacheSlot( uint requestId, QHash<QString, QString> criteria,
 void
 ChartsPlugin::chartTypes()
 {
-    /// Get possible chart type for specific chart source
+    /// Get possible chart type for specificChartsPlugin: InfoChart types returned chart source
-    tDebug() << "ChartsPlugin: InfoChart types returned!";
+    tDebug() << "Got chart type result";
     QNetworkReply* reply = qobject_cast<QNetworkReply*>( sender() );
 
     if ( reply->error() == QNetworkReply::NoError )
     {
         QJson::Parser p;
         bool ok;
-        QVariantMap res = p.parse( reply, &ok ).toMap();
+        const QVariantMap res = p.parse( reply, &ok ).toMap();
+        const QVariantMap chartObjs = res.value( "charts" ).toMap();
 
         if ( !ok )
         {
@@ -271,116 +274,80 @@ ChartsPlugin::chartTypes()
         }
 
         /// Got types, append!
-        foreach ( QVariant chart, res.value( "charts" ).toMap() )
+        const QString source = res.value( "source" ).toString();
+
+        // We'll populate charts with the data from the server
+        QVariantMap charts;
+        QString chartName;
+        if ( source == "itunes" )
         {
-             m_chartTypes.append( chart );
+            // Itunes has geographic-area based charts. So we build a breadcrumb of
-        }
+            // ITunes - Country - Albums - Top Chart Type
+            //                  - Tracks - Top Chart Type
+            QHash< QString, QVariantMap > countries;
+            foreach( const QVariant& chartObj, chartObjs.values() )
+            {
+                const QVariantMap chart = chartObj.toMap();
+                const QString id = chart.value( "id" ).toString();
+                const QString country = tr( "Country: %1" ).arg( chart.value( "geo" ).toString() );
+                QString name = chart.value( "name" ).toString();
+                const QString type = chart.value( "type" ).toString();
 
-        tDebug() << "Chart types we got:" << m_chartType;
+                if ( name.startsWith( "iTunes Store:" ) ) // truncate
-        /// Itunes have alot of country specified charts,
+                    name = name.mid( 13 );
-        /// Get those for later use
+
-        QList<QString> geos;
+                const Chart c( id, name, "album" );
-        foreach( QVariant type, m_chartTypes )
+                QList<Chart> countryTypeData = countries[ country ][ type ].value<QList<Chart> >();
+                countryTypeData.append( c );
+
+                countries[ country ].insert( type, QVariant::fromValue<QList<Chart> >( countryTypeData ) );
+            }
+
+            foreach( const QString& c, countries.keys() )
+            {
+                charts[ c ] = countries[ c ];
+//                 qDebug() << "Country has types:" << countries[ c ];
+            }
+            chartName = "iTunes";
+        } else
         {
-
+            // We'll just build:
-            if( type.toMap().value( "geo" ).isValid() )
+            // [Source] - Album - Chart Type
+            // [Source] - Track - Chart Type
+            QList< Chart > albumCharts;
+            QList< Chart > trackCharts;
+            foreach( const QVariant& chartObj, chartObjs.values() )
             {
-                geos.append( type.toMap().value( "geo" ).toString() );
+                const QVariantMap chart = chartObj.toMap();
-            }
+                const QString type = chart.value( "type" ).toString();
-        }
+                const QString id = chart.value( "id" ).toString();
-        /// We only need a unique list
+                const QString name = chart.value( "name" ).toString();
-        geos = QSet<QString>::fromList(geos).toList();
+                if ( type == "Album" )
-
+                    albumCharts.append( Chart(  id, name, "album" ) );
-        foreach( QVariant chartResource, m_chartResources )
+                else if ( type == "Track" )
-        {
+                    trackCharts.append( Chart( id, name, "tracks" ) );
-
-            QList<Chart> album_charts;
-            QList<Chart> track_charts;
-            QVariantMap charts;
-
-            if( chartResource.toString() == "itunes")
-            {
-                QVariantMap geoCharts;
-
-                foreach(QVariant country, geos)
-                {
-                    QList<Chart> geoAlbum_charts;
-                    QList<Chart> geoTrack_charts;
-
-                   foreach( QVariant type, m_chartTypes )
-                   {
-
-                        /// Itunes supplys charts based on geo, create breadcrumb for each of them
-
-                        if( type.toMap().value( "source" ).toString() == chartResource.toString()
-                        && type.toMap().value( "geo" ).isValid() )
-                        {
-
-                            if( type.toMap().value( "geo" ).toString() == country.toString() )
-                            {
-                                QString countryString = "Geo: " + type.toMap().value( "geo" ).toString().toUpper();
-
-                                if( type.toMap().value( "type" ).toString() == "Album" )
-                                {
-                                    geoAlbum_charts.append( Chart(  type.toMap().value("id").toString(), type.toMap().value("name").toString() , "album" ) );
-                                    geoCharts.insert( "Albums", QVariant::fromValue<QList<Chart> >( geoAlbum_charts ) );
-                                    charts.insert( countryString, QVariant::fromValue<QVariantMap >( geoCharts ) );
-                                }
-
-                                if( type.toMap().value( "type" ).toString() == "Track" )
-                                {
-
-                                    geoTrack_charts.append( Chart(  type.toMap().value("id").toString(), type.toMap().value("name").toString(), "tracks" ) );
-                                    geoCharts.insert( "Tracks", QVariant::fromValue<QList<Chart> >( geoTrack_charts ) );
-                                    charts.insert( countryString, QVariant::fromValue<QVariantMap >( geoCharts ) );
-
-                                }
-                            }
-                        }
-                    }
-                }
-
-            }
-            else
-            {
-                 /// Billboard, and maybe others
-                 foreach ( QVariant type, m_chartTypes )
-                 {
-
-                        /// Append each type to its parent source
-                        /// @todo Add chartType enum
-                        if ( type.toMap().value( "source" ).toString() == chartResource.toString() )
-                        {
-                            if ( type.toMap().value( "type" ).toString() == "Album" )
-                            {
-                                album_charts.append( Chart(  type.toMap().value("id").toString(), type.toMap().value("name").toString(), "album" ) );
-                                charts.insert( "Albums", QVariant::fromValue<QList<Chart> >( album_charts ) );
-                            }
-
-                            if ( type.toMap().value( "type" ).toString() == "Track" )
-                            {
-                                track_charts.append( Chart( type.toMap().value("id").toString(), type.toMap().value("name").toString(), "tracks" ) );
-                                charts.insert( "Tracks", QVariant::fromValue<QList<Chart> >( track_charts ) );
-                            }
-                        }
-                 }
             }
+            charts.insert( tr( "Albums" ), QVariant::fromValue< QList<Chart> >( albumCharts ) );
+            charts.insert( tr( "Tracks" ), QVariant::fromValue< QList<Chart> >( trackCharts ) );
 
             /// @note For displaying purposes, upper the first letter
             /// @note Remeber to lower it when fetching this!
-            QString chartName = chartResource.toString();
+            chartName = source;
             chartName[0] = chartName[0].toUpper();
-
-            /// Add the possible charts and its types to breadcrumb
-            m_allChartsMap.insert( chartName , QVariant::fromValue<QVariantMap>( charts ) );
         }
+
+        /// Add the possible charts and its types to breadcrumb
+//         qDebug() << "ADDING CHART TYPE TO CHARTS:" << chartName;
+        m_allChartsMap.insert( chartName , QVariant::fromValue<QVariantMap>( charts ) );
+
     }
     else
     {
         tLog() << "Error fetching charts:" << reply->errorString();
     }
 
-    if ( !m_cachedRequests.isEmpty() )
+    m_chartsFetchJobs--;
+    if ( !m_cachedRequests.isEmpty() && m_chartsFetchJobs == 0 )
     {
         QPair< uint, InfoRequestData > request;
         foreach ( request, m_cachedRequests )
@@ -428,12 +395,13 @@ ChartsPlugin::chartReturned()
             setChartType( None );
 
 
+        qDebug() << "Got chart returned!" << res;
         foreach ( QVariant chartR, chartResponse )
         {
             QString title, artist, album;
             QVariantMap chartMap = chartR.toMap();
 
-            if( !chartMap.isEmpty() )
+            if ( !chartMap.isEmpty() )
             {
 
                 title = chartMap.value( "track" ).toString();
@@ -442,17 +410,18 @@ ChartsPlugin::chartReturned()
                 /// Maybe we can use rank later on, to display something nice
                 /// rank = chartMap.value( "rank" ).toString();
 
-                if( chartType() == Album )
+                if ( chartType() == Album )
                 {
                      /** HACK, billboard chart returns wrong typename **/
-                    if( res.value( "source" ).toString() == "billboard" )
+                    if ( res.value( "source" ).toString() == "billboard" )
                         album = chartMap.value( "track" ).toString();
 
                     if ( album.isEmpty() && artist.isEmpty() ) // don't have enough...
                     {
                         tLog() << "Didn't get an artist and album name from chart, not enough to build a query on. Aborting" << title << album << artist;
 
-                    }else
+                    }
+                    else
                     {
                         qDebug() << Q_FUNC_INFO << album << artist;
                         ArtistAlbumPair pair;
@@ -463,14 +432,16 @@ ChartsPlugin::chartReturned()
                     }
                 }
 
-                else if( chartType() == Track )
+                else if ( chartType() == Track )
                 {
 
                     if ( title.isEmpty() && artist.isEmpty() ) // don't have enough...
                     {
                         tLog() << "Didn't get an artist and track name from charts, not enough to build a query on. Aborting" << title << artist << album;
 
-                    }else{
+                    }
+                    else
+                    {
 
                         ArtistTrackPair pair;
                         pair.artist = artist;
@@ -496,15 +467,17 @@ ChartsPlugin::chartReturned()
             returnedData["type"] = "albums";
         }
 
-    Tomahawk::InfoSystem::InfoRequestData requestData = reply->property( "requestData" ).value< Tomahawk::InfoSystem::InfoRequestData >();
+        Tomahawk::InfoSystem::InfoRequestData requestData = reply->property( "requestData" ).value< Tomahawk::InfoSystem::InfoRequestData >();
 
-    emit info(
-        reply->property( "requestId" ).toUInt(),
-        requestData,
-        returnedData
-    );
-    // TODO update cache
 
-    }else qDebug() << "Network error";
+        emit info(
+            reply->property( "requestId" ).toUInt(),
+            requestData,
+            returnedData
+        );
+        // TODO update cache
+    }
+    else
+        qDebug() << "Network error in fetching chart:" << reply->url().toString();
 
 }

src/libtomahawk/infosystem/infoplugins/generic/chartsplugin.h

@@ -67,12 +67,12 @@ private:
     void dataError( uint requestId, Tomahawk::InfoSystem::InfoRequestData requestData );
 
     QVariantList m_chartResources;
-    QVariantList m_chartTypes;
     QList<Chart> m_charts;
     ChartType m_chartType;
 
     QVariantMap m_allChartsMap;
 
+    uint m_chartsFetchJobs;
     QList< QPair< uint, InfoRequestData > > m_cachedRequests;
 
     QWeakPointer< QNetworkAccessManager > m_nam;

src/libtomahawk/infosystem/infoplugins/generic/lastfmplugin.cpp

@@ -450,6 +450,7 @@ LastFmPlugin::notInCacheSlot( uint requestId, QHash<QString, QString> criteria,
             QVariantMap result;
             result.insert( "Last.fm", QVariant::fromValue<QVariantMap>( charts ) );
 
+            tDebug() << "LASTFM RETURNING CHART LIST!";
             emit info(
                 requestId,
                 requestData,

src/libtomahawk/widgets/whatshotwidget.cpp

@@ -223,9 +223,9 @@ WhatsHotWidget::infoSystemInfo( Tomahawk::InfoSystem::InfoRequestData requestDat
                 foreach ( const Tomahawk::InfoSystem::ArtistAlbumPair& album, albums )
                 {
                     qDebug() << "Getting album" << album.album << "By" << album.artist;
-                    album_ptr albumPtr = Album::get(Artist::get( album.artist, true ), album.album );
+                    album_ptr albumPtr = Album::get( Artist::get( album.artist, true ), album.album );
 
-                    if(!albumPtr.isNull())
+                    if( !albumPtr.isNull() )
                         al << albumPtr;
 
                 }
@@ -299,6 +299,8 @@ WhatsHotWidget::leftCrumbIndexChanged( QModelIndex index )
     requestData.input = QVariant::fromValue< Tomahawk::InfoSystem::InfoCriteriaHash >( criteria );
 
     requestData.type = Tomahawk::InfoSystem::InfoChart;
+
+    qDebug() << "Making infosystem request for chart of type:" <<chartId;
     Tomahawk::InfoSystem::InfoSystem::instance()->getInfo( requestData,  20000, true );
 }
 
@@ -352,6 +354,7 @@ WhatsHotWidget::parseNode(QStandardItem* parentItem, const QString &label, const
 
         foreach( const QVariant value, dataList )
         {
+            qDebug() << "CREATED:" << value.toString();
             QStandardItem *childItem= new QStandardItem(value.toString());
             sourceItem->appendRow(childItem);
         }