Bug 231230

Summary: Podcasts are duplicated on update
Product: [Applications] amarok Reporter: Zdeněk Zikán <zdenek.zikan>
Component: PodcastAssignee: Amarok Developers <amarok-bugs-dist>
Status: RESOLVED FIXED    
Severity: normal CC: bart.cerneels, bjorn.ballard, pcbadger, per.lindstrom68
Priority: NOR    
Version: 2.3.0   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In: 2.3.1
Sentry Crash Report:
Attachments: First run
Second run after another update of the podcast

Description Zdeněk Zikán 2010-03-18 16:24:59 UTC 
Version:           2.3.0 (using 4.4.1 (KDE 4.4.1), Kubuntu packages)
Compiler:          cc
OS:                Linux (i686) release 2.6.31-17-generic

In "Podcasts" media source I right click on a podcast and select "Update channel". The episodes that are downloaded are added to the list even if they are already there, so every episode is there twice, three times etc. (depending on how many times do I click "Update channel". The same behaviour if I click "Update All". This bug was introduced in Amarok 2.3.0, in 2.2.x it was OK.
Comment 1 Sven Krohlas 2010-03-18 16:38:03 UTC 
-> over to Podcasts and Bart
Comment 2 Bart Cerneels 2010-03-18 20:42:50 UTC 
Does this happen with all channels or is there a specific one this happens with?

Please paste the url of that channel here.
Comment 3 Zdeněk Zikán 2010-03-19 10:19:31 UTC 
I tried it with these two podcasts:
http://www2.rozhlas.cz/podcast/radiozurnal.rss
http://downloads.bbc.co.uk/podcasts/worldservice/docarchive/rss.xml
and it happens with both of them (no matter whether I update them one-by-one or "Update All")
Comment 4 Sven Krohlas 2010-03-22 23:54:46 UTC 
*** Bug 231809 has been marked as a duplicate of this bug. ***
Comment 5 Bart Cerneels 2010-03-23 21:29:21 UTC 
I can't reproduce this. There might be something wrong with your database. Is this a normal setup or do you use a MySQL server?

Could you please run the next command in the script console and attach the result in a text file.

Amarok.Collection.query( "SELECT id, title, url, guid FROM podcastepisodes WHERE channel = (SELECT id FROM podcastchannels WHERE url='http://www2.rozhlas.cz/podcast/radiozurnal.rss');" );
Comment 6 Per Lindström 2010-03-24 10:04:48 UTC 
(In reply to comment #5)
> I can't reproduce this. There might be something wrong with your database. Is
> this a normal setup or do you use a MySQL server?
> 
> Could you please run the next command in the script console and attach the
> result in a text file.
> 
> Amarok.Collection.query( "SELECT id, title, url, guid FROM podcastepisodes
> WHERE channel = (SELECT id FROM podcastchannels WHERE
> url='http://www2.rozhlas.cz/podcast/radiozurnal.rss');" );

I have the same problem with podcasts from NRK (http://nrk.no/podkast/), for example http://podkast.nrk.no/program/naturtelefonen.rss. I ran the above command substituting the url, but I don't konw how to copy the text from the script console.
Comment 7 Bart Cerneels 2010-03-24 10:57:49 UTC 
(In reply to comment #6)
> I have the same problem with podcasts from NRK (http://nrk.no/podkast/), for
> example http://podkast.nrk.no/program/naturtelefonen.rss. I ran the above
> command substituting the url, but I don't konw how to copy the text from the
> script console.

Select the line and Ctrl+C. Make sure to paste it in a text file and attach it here. The content will be to long to parse in a comment.
Comment 8 Per Lindström 2010-03-24 15:20:24 UTC 
Created attachment 42228 [details]
First run
Comment 9 Per Lindström 2010-03-24 15:26:51 UTC 
Created attachment 42229 [details]
Second run after another update of the podcast
Comment 10 Zdeněk Zikán 2010-03-24 15:34:07 UTC 
It stopped doing after update to a newer version of Kubuntu package (now I have 2:2.3.0-0ubuntu1~karmic1~ppa2), so I'm not sure whether it was bug that was in Ubuntu package only or whether it was Amarok bug fixed by Ubuntu devs.
Comment 11 Sven Krohlas 2010-03-24 20:26:03 UTC 
*** Bug 232032 has been marked as a duplicate of this bug. ***
Comment 12 Bart Cerneels 2010-03-25 21:34:22 UTC 
commit d799a6ca67a3eb7ab0409948d40777ecdb70d745
Author: Bart Cerneels <bart.cerneels@kde.org>
Date:   Thu Mar 25 20:13:04 2010 +0100

    Work around KUrl string parsing for guids.
    
    trackForUrl can not be used to query by guid because urls are not allowed to have non-latin characters, spaces, percent encoded entries, capitals, etc.
    
    So we'll use a function that doesn't take a KUrl.
    
    BUG:231230

diff --git a/src/collection/umscollection/podcasts/UmsPodcastProvider.cpp b/src/collection/umscollection/podcasts/UmsPodcastProvider.cpp
index 3a8ce5b..7700a16 100644
--- a/src/collection/umscollection/podcasts/UmsPodcastProvider.cpp
+++ b/src/collection/umscollection/podcasts/UmsPodcastProvider.cpp
@@ -55,6 +55,13 @@ UmsPodcastProvider::trackForUrl( const KUrl &url )
     return TrackPtr();
 }
 
+PodcastEpisodePtr
+UmsPodcastProvider::episodeForGuid( const QString &guid )
+{
+    Q_UNUSED( guid )
+    return PodcastEpisodePtr();
+}
+
 void
 UmsPodcastProvider::addPodcast( const KUrl &url )
 {
diff --git a/src/collection/umscollection/podcasts/UmsPodcastProvider.h b/src/collection/umscollection/podcasts/UmsPodcastProvider.h
index 059ed45..f052769 100644
--- a/src/collection/umscollection/podcasts/UmsPodcastProvider.h
+++ b/src/collection/umscollection/podcasts/UmsPodcastProvider.h
@@ -36,6 +36,8 @@ class UmsPodcastProvider : public PodcastProvider
         virtual bool possiblyContainsTrack( const KUrl &url ) const;
         virtual Meta::TrackPtr trackForUrl( const KUrl &url );
 
+        virtual Meta::PodcastEpisodePtr episodeForGuid( const QString &guid );
+
         virtual void addPodcast( const KUrl &url );
 
         virtual Meta::PodcastChannelPtr addChannel( Meta::PodcastChannelPtr channel );
diff --git a/src/podcasts/PodcastProvider.h b/src/podcasts/PodcastProvider.h
index 6bda936..d670ec1 100644
--- a/src/podcasts/PodcastProvider.h
+++ b/src/podcasts/PodcastProvider.h
@@ -41,6 +41,14 @@ class AMAROK_EXPORT PodcastProvider : public Amarok::TrackProvider, public Playl
         virtual bool possiblyContainsTrack( const KUrl &url ) const = 0;
         virtual Meta::TrackPtr trackForUrl( const KUrl &url ) = 0;
 
+        /** Special function to get an episode for a given guid.
+          *
+          * note: this functions is required because KUrl does not preserve every possible guids.
+          * This means we can not use trackForUrl().
+          * Problematic guids contain non-latin characters, percent encoded parts, capitals, etc.
+          */
+        virtual Meta::PodcastEpisodePtr episodeForGuid( const QString &guid ) = 0;
+
         virtual void addPodcast( const KUrl &url ) = 0;
 
         virtual Meta::PodcastChannelPtr addChannel( Meta::PodcastChannelPtr channel ) = 0;
diff --git a/src/podcasts/PodcastReader.cpp b/src/podcasts/PodcastReader.cpp
index f609222..1722bd8 100644
--- a/src/podcasts/PodcastReader.cpp
+++ b/src/podcasts/PodcastReader.cpp
@@ -1149,15 +1149,18 @@ PodcastReader::endItem()
             m_item->setDescription( description );
         }
 
+        Meta::PodcastEpisodePtr episode;
         QString guid = m_item->guid();
-        bool useGuid = !guid.isEmpty() &&
-                       !guid.contains( "[A-Z]" ) && //KUrl only uses lowercase
-                       !guid.startsWith( '/' ); //appends file://
-
-        const KUrl trackId( useGuid ? m_item->guid() : m_item->uidUrl() );
-        Meta::PodcastEpisodePtr episode = Meta::PodcastEpisodePtr::dynamicCast(
-                                              m_podcastProvider->trackForUrl( trackId )
+        if( guid.isEmpty() )
+        {
+             episode = Meta::PodcastEpisodePtr::dynamicCast(
+                                              m_podcastProvider->trackForUrl( m_item->uidUrl() )
                                           );
+        }
+        else
+        {
+            episode = m_podcastProvider->episodeForGuid( guid );
+        }
 
         //make sure that the episode is not a bogus match. The channel has to be correct.
         // See http://bugs.kde.org/show_bug.cgi?id=227515
diff --git a/src/podcasts/sql/SqlPodcastProvider.cpp b/src/podcasts/sql/SqlPodcastProvider.cpp
index fa11604..a805867 100644
--- a/src/podcasts/sql/SqlPodcastProvider.cpp
+++ b/src/podcasts/sql/SqlPodcastProvider.cpp
@@ -170,47 +170,25 @@ SqlPodcastProvider::loadPodcasts()
     emit( updated() );
 }
 
-QString
-SqlPodcastProvider::cleanUrlOrGuid( const KUrl &url )
+SqlPodcastEpisodePtr
+SqlPodcastProvider::sqlEpisodeForString( const QString &string )
 {
-    QString decodedUrl = QUrl::fromPercentEncoding( url.url().toUtf8() );
-    return decodedUrl;
-}
+    if( string.isEmpty() )
+        return SqlPodcastEpisodePtr();
 
-bool
-SqlPodcastProvider::possiblyContainsTrack( const KUrl &url ) const
-{
     SqlStorage *sqlStorage = CollectionManager::instance()->sqlStorage();
     if( !sqlStorage )
-        return false;
-
-    QString command = "SELECT title FROM podcastepisodes WHERE guid='%1' OR url='%1' "
-                      "OR localurl='%1';";
-    command = command.arg( sqlStorage->escape( cleanUrlOrGuid( url ) ) );
-
-    QStringList dbResult = sqlStorage->query( command );
-    return !dbResult.isEmpty();
-}
-
-Meta::TrackPtr
-SqlPodcastProvider::trackForUrl( const KUrl &url )
-{
-    if( url.isEmpty() )
-        return TrackPtr();
-
-    SqlStorage *sqlStorage = CollectionManager::instance()->sqlStorage();
-    if( !sqlStorage )
-        return TrackPtr();
+        return SqlPodcastEpisodePtr();
 
     QString command = "SELECT id, url, channel, localurl, guid, "
             "title, subtitle, sequencenumber, description, mimetype, pubdate, "
             "duration, filesize, isnew FROM podcastepisodes "
             "WHERE guid='%1' OR url='%1' OR localurl='%1' ORDER BY id DESC;";
-    command = command.arg( sqlStorage->escape( cleanUrlOrGuid( url ) ) );
+    command = command.arg( sqlStorage->escape( string ) );
     QStringList dbResult = sqlStorage->query( command );
 
     if( dbResult.isEmpty() )
-        return TrackPtr();
+        return SqlPodcastEpisodePtr();
 
     int episodeId = dbResult[0].toInt();
     int channelId = dbResult[2].toInt();
@@ -229,23 +207,42 @@ SqlPodcastProvider::trackForUrl( const KUrl &url )
     {
         error() << QString( "There is a track in the database with url/guid=%1 (%2) "
                             "but there is no channel with dbId=%3 in our list!" )
-                .arg( url.url() ).arg( episodeId ).arg( channelId );
-        return TrackPtr();
+                .arg( string ).arg( episodeId ).arg( channelId );
+        return SqlPodcastEpisodePtr();
     }
 
     Meta::SqlPodcastEpisodePtr episode;
     foreach( episode, channel->sqlEpisodes() )
-    {
         if( episode->dbId() == episodeId )
-        {
-            debug() << "found it!";
-            return Meta::TrackPtr::dynamicCast( episode );
-        }
-    }
+            return episode;
 
     //The episode was found in the database but it's channel didn't have it in it's list.
     //That probably is because it's beyond the purgecount limit.
-    episode = new Meta::SqlPodcastEpisode( dbResult.mid( 0, 14 ), channel );
+    return SqlPodcastEpisodePtr( new SqlPodcastEpisode( dbResult.mid( 0, 14 ), channel ) );
+}
+
+bool
+SqlPodcastProvider::possiblyContainsTrack( const KUrl &url ) const
+{
+    SqlStorage *sqlStorage = CollectionManager::instance()->sqlStorage();
+    if( !sqlStorage )
+        return false;
+
+    QString command = "SELECT title FROM podcastepisodes WHERE guid='%1' OR url='%1' "
+                      "OR localurl='%1';";
+    command = command.arg( sqlStorage->escape( url.url() ) );
+
+    QStringList dbResult = sqlStorage->query( command );
+    return !dbResult.isEmpty();
+}
+
+Meta::TrackPtr
+SqlPodcastProvider::trackForUrl( const KUrl &url )
+{
+    if( url.isEmpty() )
+        return TrackPtr();
+
+    SqlPodcastEpisodePtr episode = sqlEpisodeForString( url.url() );
 
     return Meta::TrackPtr::dynamicCast( episode );
 }
@@ -263,6 +260,12 @@ SqlPodcastProvider::playlists()
     return playlistList;
 }
 
+Meta::PodcastEpisodePtr
+SqlPodcastProvider::episodeForGuid( const QString &guid )
+{
+    return PodcastEpisodePtr::dynamicCast( sqlEpisodeForString( guid ) );
+}
+
 void
 SqlPodcastProvider::addPodcast( const KUrl &url )
 {
diff --git a/src/podcasts/sql/SqlPodcastProvider.h b/src/podcasts/sql/SqlPodcastProvider.h
index d45c968..0bfb20b 100644
--- a/src/podcasts/sql/SqlPodcastProvider.h
+++ b/src/podcasts/sql/SqlPodcastProvider.h
@@ -50,6 +50,8 @@ class SqlPodcastProvider : public PodcastProvider
         Meta::PlaylistList playlists();
 
         //PodcastProvider methods
+        virtual Meta::PodcastEpisodePtr episodeForGuid( const QString &guid );
+
         void addPodcast( const KUrl &url );
 
         Meta::PodcastChannelPtr addChannel( Meta::PodcastChannelPtr channel );
@@ -114,9 +116,8 @@ class SqlPodcastProvider : public PodcastProvider
         void createTables() const;
         void loadPodcasts();
 
-        /** return the url as a string. Removes percent encoding if it actually has a non-url guid.
-        */
-        static QString cleanUrlOrGuid( const KUrl &url );
+        /** @arg string: a url, localUrl or guid in string form */
+        Meta::SqlPodcastEpisodePtr sqlEpisodeForString( const QString &string );
 
         void updateDatabase( int fromVersion, int toVersion );
         void fetchImage( Meta::SqlPodcastChannelPtr channel );
Comment 13 Sven Krohlas 2010-03-25 21:42:44 UTC 
This has just been fixed by Bart:

commit d799a6ca67a3eb7ab0409948d40777ecdb70d745
Author: Bart Cerneels <bart.cerneels@kde.org>
Date:   Thu Mar 25 20:13:04 2010 +0100

    Work around KUrl string parsing for guids.
    
    trackForUrl can not be used to query by guid because urls are not allowed to have non-latin characters, spaces, percent encoded entries, capitals, etc.
    
    So we'll use a function that doesn't take a KUrl.
    
    BUG:231230

diff --git a/src/collection/umscollection/podcasts/UmsPodcastProvider.cpp b/src/collection/umscollection/podcasts/UmsPodcastProvider.cpp
index 3a8ce5b..7700a16 100644
--- a/src/collection/umscollection/podcasts/UmsPodcastProvider.cpp
+++ b/src/collection/umscollection/podcasts/UmsPodcastProvider.cpp
@@ -55,6 +55,13 @@ UmsPodcastProvider::trackForUrl( const KUrl &url )
     return TrackPtr();
 }
 
+PodcastEpisodePtr
+UmsPodcastProvider::episodeForGuid( const QString &guid )
+{
+    Q_UNUSED( guid )
+    return PodcastEpisodePtr();
+}
+
 void
 UmsPodcastProvider::addPodcast( const KUrl &url )
 {
diff --git a/src/collection/umscollection/podcasts/UmsPodcastProvider.h b/src/collection/umscollection/podcasts/UmsPodcastProvider.h
index 059ed45..f052769 100644
--- a/src/collection/umscollection/podcasts/UmsPodcastProvider.h
+++ b/src/collection/umscollection/podcasts/UmsPodcastProvider.h
@@ -36,6 +36,8 @@ class UmsPodcastProvider : public PodcastProvider
         virtual bool possiblyContainsTrack( const KUrl &url ) const;
         virtual Meta::TrackPtr trackForUrl( const KUrl &url );
 
+        virtual Meta::PodcastEpisodePtr episodeForGuid( const QString &guid );
+
         virtual void addPodcast( const KUrl &url );
 
         virtual Meta::PodcastChannelPtr addChannel( Meta::PodcastChannelPtr channel );
diff --git a/src/podcasts/PodcastProvider.h b/src/podcasts/PodcastProvider.h
index 6bda936..d670ec1 100644
--- a/src/podcasts/PodcastProvider.h
+++ b/src/podcasts/PodcastProvider.h
@@ -41,6 +41,14 @@ class AMAROK_EXPORT PodcastProvider : public Amarok::TrackProvider, public Playl
         virtual bool possiblyContainsTrack( const KUrl &url ) const = 0;
         virtual Meta::TrackPtr trackForUrl( const KUrl &url ) = 0;
 
+        /** Special function to get an episode for a given guid.
+          *
+          * note: this functions is required because KUrl does not preserve every possible guids.
+          * This means we can not use trackForUrl().
+          * Problematic guids contain non-latin characters, percent encoded parts, capitals, etc.
+          */
+        virtual Meta::PodcastEpisodePtr episodeForGuid( const QString &guid ) = 0;
+
         virtual void addPodcast( const KUrl &url ) = 0;
 
         virtual Meta::PodcastChannelPtr addChannel( Meta::PodcastChannelPtr channel ) = 0;
diff --git a/src/podcasts/PodcastReader.cpp b/src/podcasts/PodcastReader.cpp
index f609222..1722bd8 100644
--- a/src/podcasts/PodcastReader.cpp
+++ b/src/podcasts/PodcastReader.cpp
@@ -1149,15 +1149,18 @@ PodcastReader::endItem()
             m_item->setDescription( description );
         }
 
+        Meta::PodcastEpisodePtr episode;
         QString guid = m_item->guid();
-        bool useGuid = !guid.isEmpty() &&
-                       !guid.contains( "[A-Z]" ) && //KUrl only uses lowercase
-                       !guid.startsWith( '/' ); //appends file://
-
-        const KUrl trackId( useGuid ? m_item->guid() : m_item->uidUrl() );
-        Meta::PodcastEpisodePtr episode = Meta::PodcastEpisodePtr::dynamicCast(
-                                              m_podcastProvider->trackForUrl( trackId )
+        if( guid.isEmpty() )
+        {
+             episode = Meta::PodcastEpisodePtr::dynamicCast(
+                                              m_podcastProvider->trackForUrl( m_item->uidUrl() )
                                           );
+        }
+        else
+        {
+            episode = m_podcastProvider->episodeForGuid( guid );
+        }
 
         //make sure that the episode is not a bogus match. The channel has to be correct.
         // See http://bugs.kde.org/show_bug.cgi?id=227515
diff --git a/src/podcasts/sql/SqlPodcastProvider.cpp b/src/podcasts/sql/SqlPodcastProvider.cpp
index fa11604..a805867 100644
--- a/src/podcasts/sql/SqlPodcastProvider.cpp
+++ b/src/podcasts/sql/SqlPodcastProvider.cpp
@@ -170,47 +170,25 @@ SqlPodcastProvider::loadPodcasts()
     emit( updated() );
 }
 
-QString
-SqlPodcastProvider::cleanUrlOrGuid( const KUrl &url )
+SqlPodcastEpisodePtr
+SqlPodcastProvider::sqlEpisodeForString( const QString &string )
 {
-    QString decodedUrl = QUrl::fromPercentEncoding( url.url().toUtf8() );
-    return decodedUrl;
-}
+    if( string.isEmpty() )
+        return SqlPodcastEpisodePtr();
 
-bool
-SqlPodcastProvider::possiblyContainsTrack( const KUrl &url ) const
-{
     SqlStorage *sqlStorage = CollectionManager::instance()->sqlStorage();
     if( !sqlStorage )
-        return false;
-
-    QString command = "SELECT title FROM podcastepisodes WHERE guid='%1' OR url='%1' "
-                      "OR localurl='%1';";
-    command = command.arg( sqlStorage->escape( cleanUrlOrGuid( url ) ) );
-
-    QStringList dbResult = sqlStorage->query( command );
-    return !dbResult.isEmpty();
-}
-
-Meta::TrackPtr
-SqlPodcastProvider::trackForUrl( const KUrl &url )
-{
-    if( url.isEmpty() )
-        return TrackPtr();
-
-    SqlStorage *sqlStorage = CollectionManager::instance()->sqlStorage();
-    if( !sqlStorage )
-        return TrackPtr();
+        return SqlPodcastEpisodePtr();
 
     QString command = "SELECT id, url, channel, localurl, guid, "
             "title, subtitle, sequencenumber, description, mimetype, pubdate, "
             "duration, filesize, isnew FROM podcastepisodes "
             "WHERE guid='%1' OR url='%1' OR localurl='%1' ORDER BY id DESC;";
-    command = command.arg( sqlStorage->escape( cleanUrlOrGuid( url ) ) );
+    command = command.arg( sqlStorage->escape( string ) );
     QStringList dbResult = sqlStorage->query( command );
 
     if( dbResult.isEmpty() )
-        return TrackPtr();
+        return SqlPodcastEpisodePtr();
 
     int episodeId = dbResult[0].toInt();
     int channelId = dbResult[2].toInt();
@@ -229,23 +207,42 @@ SqlPodcastProvider::trackForUrl( const KUrl &url )
     {
         error() << QString( "There is a track in the database with url/guid=%1 (%2) "
                             "but there is no channel with dbId=%3 in our list!" )
-                .arg( url.url() ).arg( episodeId ).arg( channelId );
-        return TrackPtr();
+                .arg( string ).arg( episodeId ).arg( channelId );
+        return SqlPodcastEpisodePtr();
     }
 
     Meta::SqlPodcastEpisodePtr episode;
     foreach( episode, channel->sqlEpisodes() )
-    {
         if( episode->dbId() == episodeId )
-        {
-            debug() << "found it!";
-            return Meta::TrackPtr::dynamicCast( episode );
-        }
-    }
+            return episode;
 
     //The episode was found in the database but it's channel didn't have it in it's list.
     //That probably is because it's beyond the purgecount limit.
-    episode = new Meta::SqlPodcastEpisode( dbResult.mid( 0, 14 ), channel );
+    return SqlPodcastEpisodePtr( new SqlPodcastEpisode( dbResult.mid( 0, 14 ), channel ) );
+}
+
+bool
+SqlPodcastProvider::possiblyContainsTrack( const KUrl &url ) const
+{
+    SqlStorage *sqlStorage = CollectionManager::instance()->sqlStorage();
+    if( !sqlStorage )
+        return false;
+
+    QString command = "SELECT title FROM podcastepisodes WHERE guid='%1' OR url='%1' "
+                      "OR localurl='%1';";
+    command = command.arg( sqlStorage->escape( url.url() ) );
+
+    QStringList dbResult = sqlStorage->query( command );
+    return !dbResult.isEmpty();
+}
+
+Meta::TrackPtr
+SqlPodcastProvider::trackForUrl( const KUrl &url )
+{
+    if( url.isEmpty() )
+        return TrackPtr();
+
+    SqlPodcastEpisodePtr episode = sqlEpisodeForString( url.url() );
 
     return Meta::TrackPtr::dynamicCast( episode );
 }
@@ -263,6 +260,12 @@ SqlPodcastProvider::playlists()
     return playlistList;
 }
 
+Meta::PodcastEpisodePtr
+SqlPodcastProvider::episodeForGuid( const QString &guid )
+{
+    return PodcastEpisodePtr::dynamicCast( sqlEpisodeForString( guid ) );
+}
+
 void
 SqlPodcastProvider::addPodcast( const KUrl &url )
 {
diff --git a/src/podcasts/sql/SqlPodcastProvider.h b/src/podcasts/sql/SqlPodcastProvider.h
index d45c968..0bfb20b 100644
--- a/src/podcasts/sql/SqlPodcastProvider.h
+++ b/src/podcasts/sql/SqlPodcastProvider.h
@@ -50,6 +50,8 @@ class SqlPodcastProvider : public PodcastProvider
         Meta::PlaylistList playlists();
 
         //PodcastProvider methods
+        virtual Meta::PodcastEpisodePtr episodeForGuid( const QString &guid );
+
         void addPodcast( const KUrl &url );
 
         Meta::PodcastChannelPtr addChannel( Meta::PodcastChannelPtr channel );
@@ -114,9 +116,8 @@ class SqlPodcastProvider : public PodcastProvider
         void createTables() const;
         void loadPodcasts();
 
-        /** return the url as a string. Removes percent encoding if it actually has a non-url guid.
-        */
-        static QString cleanUrlOrGuid( const KUrl &url );
+        /** @arg string: a url, localUrl or guid in string form */
+        Meta::SqlPodcastEpisodePtr sqlEpisodeForString( const QString &string );
 
         void updateDatabase( int fromVersion, int toVersion );
         void fetchImage( Meta::SqlPodcastChannelPtr channel );
Comment 14 Bart Cerneels 2010-03-25 22:27:58 UTC 
commit 839538ae580be93d4ab6260b3b3825ed584b02c2
Author: Bart Cerneels <bart.cerneels@kde.org>
Date:   Thu Mar 25 20:13:04 2010 +0100

    Work around KUrl string parsing for guids.
    
    trackForUrl can not be used to query by guid because urls are not allowed to have non-latin characters, spaces, percent encoded entries, capitals, etc.
    
    So we'll use a function that doesn't take a KUrl.
    
    BUG:231230

diff --git a/src/collection/umscollection/podcasts/UmsPodcastProvider.cpp b/src/collection/umscollection/podcasts/UmsPodcastProvider.cpp
index 3a8ce5b..7700a16 100644
--- a/src/collection/umscollection/podcasts/UmsPodcastProvider.cpp
+++ b/src/collection/umscollection/podcasts/UmsPodcastProvider.cpp
@@ -55,6 +55,13 @@ UmsPodcastProvider::trackForUrl( const KUrl &url )
     return TrackPtr();
 }
 
+PodcastEpisodePtr
+UmsPodcastProvider::episodeForGuid( const QString &guid )
+{
+    Q_UNUSED( guid )
+    return PodcastEpisodePtr();
+}
+
 void
 UmsPodcastProvider::addPodcast( const KUrl &url )
 {
diff --git a/src/collection/umscollection/podcasts/UmsPodcastProvider.h b/src/collection/umscollection/podcasts/UmsPodcastProvider.h
index 059ed45..f052769 100644
--- a/src/collection/umscollection/podcasts/UmsPodcastProvider.h
+++ b/src/collection/umscollection/podcasts/UmsPodcastProvider.h
@@ -36,6 +36,8 @@ class UmsPodcastProvider : public PodcastProvider
         virtual bool possiblyContainsTrack( const KUrl &url ) const;
         virtual Meta::TrackPtr trackForUrl( const KUrl &url );
 
+        virtual Meta::PodcastEpisodePtr episodeForGuid( const QString &guid );
+
         virtual void addPodcast( const KUrl &url );
 
         virtual Meta::PodcastChannelPtr addChannel( Meta::PodcastChannelPtr channel );
diff --git a/src/podcasts/PodcastProvider.h b/src/podcasts/PodcastProvider.h
index 6bda936..d670ec1 100644
--- a/src/podcasts/PodcastProvider.h
+++ b/src/podcasts/PodcastProvider.h
@@ -41,6 +41,14 @@ class AMAROK_EXPORT PodcastProvider : public Amarok::TrackProvider, public Playl
         virtual bool possiblyContainsTrack( const KUrl &url ) const = 0;
         virtual Meta::TrackPtr trackForUrl( const KUrl &url ) = 0;
 
+        /** Special function to get an episode for a given guid.
+          *
+          * note: this functions is required because KUrl does not preserve every possible guids.
+          * This means we can not use trackForUrl().
+          * Problematic guids contain non-latin characters, percent encoded parts, capitals, etc.
+          */
+        virtual Meta::PodcastEpisodePtr episodeForGuid( const QString &guid ) = 0;
+
         virtual void addPodcast( const KUrl &url ) = 0;
 
         virtual Meta::PodcastChannelPtr addChannel( Meta::PodcastChannelPtr channel ) = 0;
diff --git a/src/podcasts/PodcastReader.cpp b/src/podcasts/PodcastReader.cpp
index f609222..1722bd8 100644
--- a/src/podcasts/PodcastReader.cpp
+++ b/src/podcasts/PodcastReader.cpp
@@ -1149,15 +1149,18 @@ PodcastReader::endItem()
             m_item->setDescription( description );
         }
 
+        Meta::PodcastEpisodePtr episode;
         QString guid = m_item->guid();
-        bool useGuid = !guid.isEmpty() &&
-                       !guid.contains( "[A-Z]" ) && //KUrl only uses lowercase
-                       !guid.startsWith( '/' ); //appends file://
-
-        const KUrl trackId( useGuid ? m_item->guid() : m_item->uidUrl() );
-        Meta::PodcastEpisodePtr episode = Meta::PodcastEpisodePtr::dynamicCast(
-                                              m_podcastProvider->trackForUrl( trackId )
+        if( guid.isEmpty() )
+        {
+             episode = Meta::PodcastEpisodePtr::dynamicCast(
+                                              m_podcastProvider->trackForUrl( m_item->uidUrl() )
                                           );
+        }
+        else
+        {
+            episode = m_podcastProvider->episodeForGuid( guid );
+        }
 
         //make sure that the episode is not a bogus match. The channel has to be correct.
         // See http://bugs.kde.org/show_bug.cgi?id=227515
diff --git a/src/podcasts/sql/SqlPodcastProvider.cpp b/src/podcasts/sql/SqlPodcastProvider.cpp
index fa11604..a805867 100644
--- a/src/podcasts/sql/SqlPodcastProvider.cpp
+++ b/src/podcasts/sql/SqlPodcastProvider.cpp
@@ -170,47 +170,25 @@ SqlPodcastProvider::loadPodcasts()
     emit( updated() );
 }
 
-QString
-SqlPodcastProvider::cleanUrlOrGuid( const KUrl &url )
+SqlPodcastEpisodePtr
+SqlPodcastProvider::sqlEpisodeForString( const QString &string )
 {
-    QString decodedUrl = QUrl::fromPercentEncoding( url.url().toUtf8() );
-    return decodedUrl;
-}
+    if( string.isEmpty() )
+        return SqlPodcastEpisodePtr();
 
-bool
-SqlPodcastProvider::possiblyContainsTrack( const KUrl &url ) const
-{
     SqlStorage *sqlStorage = CollectionManager::instance()->sqlStorage();
     if( !sqlStorage )
-        return false;
-
-    QString command = "SELECT title FROM podcastepisodes WHERE guid='%1' OR url='%1' "
-                      "OR localurl='%1';";
-    command = command.arg( sqlStorage->escape( cleanUrlOrGuid( url ) ) );
-
-    QStringList dbResult = sqlStorage->query( command );
-    return !dbResult.isEmpty();
-}
-
-Meta::TrackPtr
-SqlPodcastProvider::trackForUrl( const KUrl &url )
-{
-    if( url.isEmpty() )
-        return TrackPtr();
-
-    SqlStorage *sqlStorage = CollectionManager::instance()->sqlStorage();
-    if( !sqlStorage )
-        return TrackPtr();
+        return SqlPodcastEpisodePtr();
 
     QString command = "SELECT id, url, channel, localurl, guid, "
             "title, subtitle, sequencenumber, description, mimetype, pubdate, "
             "duration, filesize, isnew FROM podcastepisodes "
             "WHERE guid='%1' OR url='%1' OR localurl='%1' ORDER BY id DESC;";
-    command = command.arg( sqlStorage->escape( cleanUrlOrGuid( url ) ) );
+    command = command.arg( sqlStorage->escape( string ) );
     QStringList dbResult = sqlStorage->query( command );
 
     if( dbResult.isEmpty() )
-        return TrackPtr();
+        return SqlPodcastEpisodePtr();
 
     int episodeId = dbResult[0].toInt();
     int channelId = dbResult[2].toInt();
@@ -229,23 +207,42 @@ SqlPodcastProvider::trackForUrl( const KUrl &url )
     {
         error() << QString( "There is a track in the database with url/guid=%1 (%2) "
                             "but there is no channel with dbId=%3 in our list!" )
-                .arg( url.url() ).arg( episodeId ).arg( channelId );
-        return TrackPtr();
+                .arg( string ).arg( episodeId ).arg( channelId );
+        return SqlPodcastEpisodePtr();
     }
 
     Meta::SqlPodcastEpisodePtr episode;
     foreach( episode, channel->sqlEpisodes() )
-    {
         if( episode->dbId() == episodeId )
-        {
-            debug() << "found it!";
-            return Meta::TrackPtr::dynamicCast( episode );
-        }
-    }
+            return episode;
 
     //The episode was found in the database but it's channel didn't have it in it's list.
     //That probably is because it's beyond the purgecount limit.
-    episode = new Meta::SqlPodcastEpisode( dbResult.mid( 0, 14 ), channel );
+    return SqlPodcastEpisodePtr( new SqlPodcastEpisode( dbResult.mid( 0, 14 ), channel ) );
+}
+
+bool
+SqlPodcastProvider::possiblyContainsTrack( const KUrl &url ) const
+{
+    SqlStorage *sqlStorage = CollectionManager::instance()->sqlStorage();
+    if( !sqlStorage )
+        return false;
+
+    QString command = "SELECT title FROM podcastepisodes WHERE guid='%1' OR url='%1' "
+                      "OR localurl='%1';";
+    command = command.arg( sqlStorage->escape( url.url() ) );
+
+    QStringList dbResult = sqlStorage->query( command );
+    return !dbResult.isEmpty();
+}
+
+Meta::TrackPtr
+SqlPodcastProvider::trackForUrl( const KUrl &url )
+{
+    if( url.isEmpty() )
+        return TrackPtr();
+
+    SqlPodcastEpisodePtr episode = sqlEpisodeForString( url.url() );
 
     return Meta::TrackPtr::dynamicCast( episode );
 }
@@ -263,6 +260,12 @@ SqlPodcastProvider::playlists()
     return playlistList;
 }
 
+Meta::PodcastEpisodePtr
+SqlPodcastProvider::episodeForGuid( const QString &guid )
+{
+    return PodcastEpisodePtr::dynamicCast( sqlEpisodeForString( guid ) );
+}
+
 void
 SqlPodcastProvider::addPodcast( const KUrl &url )
 {
diff --git a/src/podcasts/sql/SqlPodcastProvider.h b/src/podcasts/sql/SqlPodcastProvider.h
index d45c968..0bfb20b 100644
--- a/src/podcasts/sql/SqlPodcastProvider.h
+++ b/src/podcasts/sql/SqlPodcastProvider.h
@@ -50,6 +50,8 @@ class SqlPodcastProvider : public PodcastProvider
         Meta::PlaylistList playlists();
 
         //PodcastProvider methods
+        virtual Meta::PodcastEpisodePtr episodeForGuid( const QString &guid );
+
         void addPodcast( const KUrl &url );
 
         Meta::PodcastChannelPtr addChannel( Meta::PodcastChannelPtr channel );
@@ -114,9 +116,8 @@ class SqlPodcastProvider : public PodcastProvider
         void createTables() const;
         void loadPodcasts();
 
-        /** return the url as a string. Removes percent encoding if it actually has a non-url guid.
-        */
-        static QString cleanUrlOrGuid( const KUrl &url );
+        /** @arg string: a url, localUrl or guid in string form */
+        Meta::SqlPodcastEpisodePtr sqlEpisodeForString( const QString &string );
 
         void updateDatabase( int fromVersion, int toVersion );
         void fetchImage( Meta::SqlPodcastChannelPtr channel );