commit e69b286cfbcc970f324fd2418c1783e4d28e62da
branch master
Author: Sven Krohlas <email address hidden>
Date:   Sat Jan 8 13:02:12 2011 +0100

    Make NetworkAccessManagerProxy::getData honor redirects
    
    Patch by: Martin Blumenstingl
    
    Should fix the BBC and Free Music Charts scripts, maybe others, too.
    
    BUG: 261839

diff --git a/src/covermanager/CoverFetcher.cpp b/src/covermanager/CoverFetcher.cpp
index b21dd87..6ec1ccd 100644
--- a/src/covermanager/CoverFetcher.cpp
+++ b/src/covermanager/CoverFetcher.cpp
@@ -66,6 +66,9 @@ CoverFetcher::CoverFetcher()
     connect( m_queue, SIGNAL(fetchUnitAdded(CoverFetchUnit::Ptr)),
                       SLOT(slotFetch(CoverFetchUnit::Ptr)) );
     s_instance = this;
+
+    connect( The::networkAccessManager(), SIGNAL( requestRedirected( QNetworkReply*, QNetworkReply* ) ),
+             this, SLOT( fetchRequestRedirected( QNetworkReply*, QNetworkReply* ) ) );
 }
 
 CoverFetcher::~CoverFetcher()
@@ -312,6 +315,32 @@ CoverFetcher::slotDialogFinished()
 }
 
 void
+CoverFetcher::fetchRequestRedirected( QNetworkReply *oldReply,
+                                      QNetworkReply *newReply )
+{
+    KUrl oldUrl = oldReply->request().url();
+    KUrl newUrl = newReply->request().url();
+
+    // Since we were redirected we have to check if the redirect
+    // was for one of our URLs and if the new URL is not handled
+    // already.
+    if( m_urls.contains( oldUrl ) && !m_urls.contains( newUrl ) )
+    {
+        // Get the unit for the old URL.
+        CoverFetchUnit::Ptr unit = m_urls.value( oldUrl );
+
+        // Add the unit with the new URL and remove the old one.
+        m_urls.insert( newUrl, unit );
+        m_urls.remove( oldUrl );
+
+        // If the unit is an interactive one we have to incidate that we're
+        // still fetching the cover.
+        if( unit->isInteractive() )
+            Amarok::Components::logger()->newProgressOperation( newReply, i18n( "Fetching Cover" ) );
+    }
+}
+
+void
 CoverFetcher::showCover( const CoverFetchUnit::Ptr &unit,
                          const QPixmap &cover,
                          const CoverFetch::Metadata &data )
diff --git a/src/covermanager/CoverFetcher.h b/src/covermanager/CoverFetcher.h
index 9dc2f2e..39a1487 100644
--- a/src/covermanager/CoverFetcher.h
+++ b/src/covermanager/CoverFetcher.h
@@ -67,6 +67,9 @@ private slots:
     /// Cover found dialog is closed by the user
     void slotDialogFinished();
 
+    /// The fetch request was redirected.
+    void fetchRequestRedirected( QNetworkReply *oldReply, QNetworkReply *newReply );
+
 private:
     static CoverFetcher* s_instance;
     CoverFetcher();
diff --git a/src/network/NetworkAccessManagerProxy.cpp b/src/network/NetworkAccessManagerProxy.cpp
index 9a0e763..d5e1d81 100644
--- a/src/network/NetworkAccessManagerProxy.cpp
+++ b/src/network/NetworkAccessManagerProxy.cpp
@@ -66,38 +66,57 @@ public:
     {
         Q_Q( NetworkAccessManagerProxy );
         QNetworkReply *reply = static_cast<QNetworkReply*>( q->sender() );
+
         KUrl url = reply->request().url();
         QList<CallBackData*> callbacks = urlMap.values( url );
         QByteArray data = reply->readAll();
         data.detach(); // detach so the bytes are not deleted before methods are invoked
         foreach( const CallBackData *cb, callbacks )
         {
-            QByteArray sig = QMetaObject::normalizedSignature( cb->method );
-            sig.remove( 0, 1 ); // remove first char, which is the member code (see qobjectdefs.h)
-                                // and let Qt's meta object system handle the rest.
-            if( cb->receiver )
+            // There may have been a redirect.
+            KUrl redirectUrl = q->getRedirectUrl( reply );
+
+            // Check if there's no redirect.
+            if( redirectUrl.isEmpty() )
             {
-                bool success( false );
-                const QMetaObject *mo = cb->receiver.data()->metaObject();
-                int methodIndex = mo->indexOfSlot( sig );
-                if( methodIndex != -1 )
+                QByteArray sig = QMetaObject::normalizedSignature( cb->method );
+                sig.remove( 0, 1 ); // remove first char, which is the member code (see qobjectdefs.h)
+                                    // and let Qt's meta object system handle the rest.
+                if( cb->receiver )
                 {
-                    Error err = { reply->error(), reply->errorString() };
-                    QMetaMethod method = mo->method( methodIndex );
-                    success = method.invoke( cb->receiver.data(),
-                                             cb->type,
-                                             Q_ARG( KUrl, reply->request().url() ),
-                                             Q_ARG( QByteArray, data ),
-                                             Q_ARG( NetworkAccessManagerProxy::Error, err ) );
+                    bool success( false );
+                    const QMetaObject *mo = cb->receiver.data()->metaObject();
+                    int methodIndex = mo->indexOfSlot( sig );
+                    if( methodIndex != -1 )
+                    {
+                        Error err = { reply->error(), reply->errorString() };
+                        QMetaMethod method = mo->method( methodIndex );
+                        success = method.invoke( cb->receiver.data(),
+                                                cb->type,
+                                                Q_ARG( KUrl, reply->request().url() ),
+                                                Q_ARG( QByteArray, data ),
+                                                Q_ARG( NetworkAccessManagerProxy::Error, err ) );
+                    }
+
+                    if( !success )
+                    {
+                        debug() << QString( "Failed to invoke method %1 of %2" )
+                            .arg( QString(sig) ).arg( mo->className() );
+                    }
                 }
+            }
+            else
+            {
+                debug() << "the server is redirecting the request to: " << redirectUrl;
 
-                if( !success )
-                {
-                    debug() << QString( "Failed to invoke method %1 of %2" )
-                        .arg( QString(sig) ).arg( mo->className() );
-                }
+                // Let's try to fetch the data again, but this time from the new url.
+                QNetworkReply *newReply = q->getData( redirectUrl, cb->receiver.data(), cb->method, cb->type );
+
+                emit q->requestRedirected( url, redirectUrl );
+                emit q->requestRedirected( reply, newReply );
             }
         }
+
         qDeleteAll( callbacks );
         urlMap.remove( url );
         reply->deleteLater();
@@ -209,6 +228,31 @@ NetworkAccessManagerProxy::abortGet( const KUrl &url )
     return removed;
 }
 
+KUrl
+NetworkAccessManagerProxy::getRedirectUrl( QNetworkReply *reply )
+{
+    KUrl targetUrl;
+
+    // Get the original URL.
+    KUrl originalUrl = reply->request().url();
+
+    // Get the redirect attribute.
+    QVariant redirectAttribute = reply->attribute( QNetworkRequest::RedirectionTargetAttribute );
+
+    // Get the redirect URL from the attribute.
+    KUrl redirectUrl = KUrl( redirectAttribute.toUrl() );
+
+    // If the redirect URL is valid and if it differs from the original
+    // URL then we return the redirect URL. Otherwise an empty URL will
+    // be returned.
+    if( !redirectUrl.isEmpty() && redirectUrl != originalUrl )
+    {
+        targetUrl = redirectUrl;
+    }
+
+    return targetUrl;
+}
+
 void
 NetworkAccessManagerProxy::slotError( QObject *obj )
 {
diff --git a/src/network/NetworkAccessManagerProxy.h b/src/network/NetworkAccessManagerProxy.h
index 045ded3..4d7a7ef 100644
--- a/src/network/NetworkAccessManagerProxy.h
+++ b/src/network/NetworkAccessManagerProxy.h
@@ -55,6 +55,7 @@ public:
      * Gets the contents of the target @p url. It is a convenience wrapper
      * around QNetworkAccessManager::get() where the user supplies a
      * slot @p method to be called when the content is retrieved.
+     * NOTE: On redirects requestRedirected is emitted.
      *
      * @param url the url to get the content from.
      * @param receiver the receiver object to call @p method on.
@@ -68,11 +69,27 @@ public:
     int abortGet( const KUrl &url );
     int abortGet( const KUrl::List &urls );
 
+    /**
+     * Gets the URL to which a server redirects the request.
+     * An empty KUrl will be returned if the request was not redirected.
+     *
+     * @param reply The QNetworkReply which contains all information about
+     *              the reply from the server.
+     *
+     * @return The URL to which the server redirected the request or an empty
+     *         URL if there was no redirect.
+     */
+    KUrl getRedirectUrl( QNetworkReply *reply );
+
 #ifdef DEBUG_BUILD_TYPE
     NetworkAccessViewer *networkAccessViewer();
     void setNetworkAccessViewer( NetworkAccessViewer *viewer );
 #endif // DEBUG_BUILD_TYPE
 
+Q_SIGNALS:
+    void requestRedirected( const KUrl &sourceUrl, const KUrl &targetUrl );
+    void requestRedirected( QNetworkReply* oldReply, QNetworkReply *newReply );
+
 public slots:
     void slotError( QObject *reply );
 
diff --git a/src/scriptengine/AmarokNetworkScript.cpp b/src/scriptengine/AmarokNetworkScript.cpp
index 1742f41..c37764d 100644
--- a/src/scriptengine/AmarokNetworkScript.cpp
+++ b/src/scriptengine/AmarokNetworkScript.cpp
@@ -123,25 +123,41 @@ Downloader::init( QScriptContext* context, QScriptEngine* engine, bool stringRes
 AmarokDownloadHelper::AmarokDownloadHelper()
 {
     s_instance = this;
+    connect( The::networkAccessManager(), SIGNAL( requestRedirected( const KUrl&, const KUrl& ) ),
+             this, SLOT( requestRedirected( const KUrl&, const KUrl& ) ) );
 }
 
 void
 AmarokDownloadHelper::newStringDownload( const KUrl &url, QScriptEngine* engine, QScriptValue obj, QString encoding )
 {
-    m_values[ url ] = obj;
-    m_engines[ url ] = engine;
     m_encodings[ url ] = encoding;
-    The::networkAccessManager()->getData( url, this,
-         SLOT(resultString(KUrl,QByteArray,NetworkAccessManagerProxy::Error)) );
+    newDownload( url, engine, obj, SLOT( resultString( KUrl, QByteArray, NetworkAccessManagerProxy::Error) ) );
 }
 
 void
 AmarokDownloadHelper::newDataDownload( const KUrl &url, QScriptEngine* engine, QScriptValue obj )
 {
+    newDownload( url, engine, obj, SLOT( resultData( KUrl, QByteArray, NetworkAccessManagerProxy::Error) ) );
+}
+
+void
+AmarokDownloadHelper::newDownload( const KUrl &url, QScriptEngine* engine, QScriptValue obj, const char *slot )
+{
     m_values[ url ] = obj;
     m_engines[ url ] = engine;
-    The::networkAccessManager()->getData( url, this,
-         SLOT(resultData(KUrl,QByteArray,NetworkAccessManagerProxy::Error)) );
+
+    The::networkAccessManager()->getData( url, this, slot );
+}
+
+void
+AmarokDownloadHelper::requestRedirected( const KUrl &sourceUrl, const KUrl &targetUrl )
+{
+    DEBUG_BLOCK
+
+    // Move all entries from "url" to "targetUrl".
+    updateUrl< QScriptEngine* >( m_engines, sourceUrl, targetUrl );
+    updateUrl< QScriptValue >( m_values, sourceUrl, targetUrl );
+    updateUrl< QString >( m_encodings, sourceUrl, targetUrl );
 }
 
 void
diff --git a/src/scriptengine/AmarokNetworkScript.h b/src/scriptengine/AmarokNetworkScript.h
index 342cc2a..a23beca 100644
--- a/src/scriptengine/AmarokNetworkScript.h
+++ b/src/scriptengine/AmarokNetworkScript.h
@@ -73,6 +73,8 @@ public:
 private slots:
     void resultString( const KUrl &url, QByteArray data, NetworkAccessManagerProxy::Error e );
     void resultData( const KUrl &url, QByteArray data, NetworkAccessManagerProxy::Error e );
+
+    void requestRedirected( const KUrl &sourceUrl, const KUrl &targetUrl );
     
 private:
     QHash< KUrl, QScriptEngine* > m_engines;
@@ -80,6 +82,33 @@ private:
     QHash< KUrl, QString > m_encodings;
 
     void cleanUp( const KUrl &url );
+    void newDownload( const KUrl &url, QScriptEngine* engine, QScriptValue obj, const char *slot );
+
+    /**
+     * Template function which updates the given @p sourceUrl to the given
+     * @p targetUrl. All entries in the given hash will be copied.
+     * The old entries will be removed.
+     *
+     * @param hash The hash which contains all elements.
+     * @param sourceUrl The old URL (= the old key).
+     * @param targetUrl The new URL (= the new key).
+     */
+    template<typename T> void updateUrl( QHash< KUrl, T > &hash, const KUrl &sourceUrl, const KUrl &targetUrl )
+    {
+        // Get all entries with the source URL as key.
+        QList< T > data = hash.values( sourceUrl );
+
+        foreach( T entry, data )
+        {
+            // Copy each entry to a new one with the
+            // new URL as key.
+            hash[ targetUrl ] = entry;
+        }
+
+        // Remove all entries which are still pointing
+        // to the source URL.
+        hash.remove( sourceUrl );
+    };
 };
 
 #endif