Merge lp:~etherpulse/podbird/duplicated_downloads into lp:podbird/devel

[Eran Uzan]

Fix for download duplication when restarting Podbird.

Bug: lp:1637657

[Eran Uzan (etherpulse)]

This will create a conflict with my other MP to implement lp:1456300 ...
But i`ll be happy to fix it if needed :)

[Nekhelesh Ramananthan (nik90)]

Time to review this. So I get most of the logic here. Before Podbird downloads an episode, it checks if the guid is already present in DownloadManager's list. What I don't get is why that delay timer is required. Can you explain that? Is it because DownloadManager takes a few ms to load and so this check needs to be delayed until that happens?

review: Needs Information

[Nekhelesh Ramananthan (nik90)]

Here in,

+ var item = rs2.rows.item(j);
+ if (!item.downloadedfile && !item.listened && item.audiourl && !item.queued ) {
+ podbird.downloadEpisode(item.image, item.name, item.guid, item.audiourl )
+ tx.executeSql("UPDATE Episode SET queued=1 WHERE guid = ?", [item.guid]);

Here I think item can be better renamed as episode don't you think?

[Eran Uzan (etherpulse)]

Hey :)

Exactly, I know the timer is ugly but for some reason all the onCompleted events happen before the downloads list is populated so the downloads list is empty when we add episodes and theres nothing to compare against.
I couldn't find any way around it (checked a lots of onCompleted and other single shot events :/ )
I think I can decrease the timer delay to even 1 millisecond and it will still work my guess it`s some kind of single UI thread issue.

And yeah episode will make more sense i`ll change it.. thanks :)

[lp:~etherpulse/podbird/duplicated_downloads]

156. By Eran Uzan on 2016-11-15

Changed variable name to make the code clearer

[Eran Uzan (etherpulse)]

Hey ,
I identified an issue with the version on this branch it seems that the episode the are marked for download don['t have the podcast image assigned to them.

I`l investigate and come back to you in any case i suggest not to merge this branch for now...

[Eran Uzan (etherpulse)]

OK I've "investigated" and found out.. that I`m stupid... I used episode.image instead of podcast.image :)

[lp:~etherpulse/podbird/duplicated_downloads]

157. By Eran Uzan on 2016-11-19

Fix regression that caused a missing podcast image on the download tab

[Nekhelesh Ramananthan (nik90)]

Please bear with me while I try to brainstorming an idea here, I keep wondering why we have the following code outside of the DownloadManager,

8 - if (!NetworkingStatus.online || settings.maxEpisodeDownload === -1) {
9 - console.log("[LOG]: Skipped autodownloading of new episodes...")
10 - console.log("[LOG]: Online connectivity: " + NetworkingStatus.online)
11 - console.log("[LOG]: User settings (maxEpisodeDownload): " + settings.maxEpisodeDownload)
12 - } else {
13 - Podcasts.autoDownloadEpisodes(settings.maxEpisodeDownload)

Why can't we move this code insider the DownloadManger onCompleted{} scope? I mean, you anyway can't autodownload episodes before the DownloadManager as finished loading. So couldn't we wait for DownloadManager and then it runs the autodownloader code? This might even help the Timer.

Can you give that a try?

review: Needs Information

[Eran Uzan (etherpulse)]

Giving it at try! :)

[Eran Uzan (etherpulse)]

Hmm it seem that the DownloadManager.onCompleted signal happen even earlier then the MainView onCompleted signal it seems that any part of the MainView recevied the onCompleted signal before the main view so it doesn't really works.

I did find that if I attach it to the onStateChanged of the Qt.application it will work however this as the side effect of checking for new downloads every time podbird is brught to the front instead of a single time at the application lunch.

I`ll keep searching for other events maybe i should dynamically load the DownloadManager or some other method...

[Nekhelesh Ramananthan (nik90)]

Hi Eran, I am going to go ahead and merge this. We can come up with a better method later. Thanks for the patch and noticing the bug in the first place.

review: Approve

[Nekhelesh Ramananthan (nik90)]

Hmm on second thought, I am getting code conflicts with devel branch. Can you merge lp:podbird/devel, fix the conflicts and update it. I will then merge it asap.

review: Needs Fixing

[Eran Uzan (etherpulse)]

Will do once i get home.

[lp:~etherpulse/podbird/duplicated_downloads]

158. By Eran Uzan on 2016-11-28

Merged with lp:podbird/devel

=== modified file 'app/podbird.qml'
--- app/podbird.qml	2016-11-16 21:49:13 +0000
+++ app/podbird.qml	2016-11-28 09:51:07 +0000
@@ -78,12 +78,21 @@
             settings.lastCheck = today
         }
 
-        if (!NetworkingStatus.online || settings.maxEpisodeDownload === -1) {
-            console.log("[LOG]: Skipped autodownloading of new episodes...")
-            console.log("[LOG]: Online connectivity: " + NetworkingStatus.online)
-            console.log("[LOG]: User settings (maxEpisodeDownload): " + settings.maxEpisodeDownload)
-        } else {
-            Podcasts.autoDownloadEpisodes(settings.maxEpisodeDownload)
+        delayStartTimer.start();
+    }
+
+    Timer {
+        id: delayStartTimer
+        interval: 500
+        repeat: false
+        onTriggered: {
+            if (!NetworkingStatus.online || podbird.settings.maxEpisodeDownload === -1) {
+                console.log("[LOG]: Skipped autodownloading of new episodes...")
+                console.log("[LOG]: Online connectivity: " + NetworkingStatus.online)
+                console.log("[LOG]: User settings (maxEpisodeDownload): " + podbird.settings.maxEpisodeDownload)
+            } else {
+                Podcasts.autoDownloadEpisodes(podbird.settings.maxEpisodeDownload)
+            }
         }
     }
 
@@ -173,7 +182,12 @@
     }
 
     function downloadEpisode(image, title, guid, url, disableMobileDownload) {
-        var singleDownload = singleDownloadComponent.createObject(podbird, {"image": image, "title": title, "guid": guid , allowMobileDownload : !disableMobileDownload })
+        if(downloader.isDownloadInQueue(guid)) {
+            console.log("[LOG]: Download with GUID of :"+guid+ " is already in the download queue.")
+            return false;
+        }
+
+        var singleDownload = singleDownloadComponent.createObject(podbird, {"image": image, "title": title, "guid": guid, allowMobileDownload : !disableMobileDownload })
         singleDownload.download(url)
     }
 
@@ -184,6 +198,16 @@
         property int progress: downloads.length > 0 ? downloads[0].progress : 0
 
         cleanDownloads: true
+
+        function isDownloadInQueue ( guid ) {
+            for( var i=0; i < downloads.length; i++) {
+                if( downloads[i].metadata.custom.guid && guid === downloads[i].metadata.custom.guid) {
+                    return true ;
+                }
+            }
+            return false;
+        }
+
         onDownloadFinished: {
             var db = Podcasts.init();
             var finalLocation = fileManager.saveDownload(path);

=== modified file 'app/podcasts.js'
--- app/podcasts.js	2016-10-24 19:08:35 +0000
+++ app/podcasts.js	2016-11-28 09:51:07 +0000
@@ -356,12 +356,13 @@
         var rs = tx.executeSql("SELECT rowid, * FROM Podcast ORDER BY name ASC");
         for (var i=0; i < rs.rows.length; i++) {
             var podcast = rs.rows.item(i);
-            var rs2 = tx.executeSql("SELECT rowid, * FROM Episode WHERE podcast=? ORDER BY published DESC", [rs.rows.item(i).rowid]);
+            var rs2 = tx.executeSql("SELECT rowid, * FROM Episode WHERE podcast=? ORDER BY published DESC", [podcast.rowid]);
             var loopCount = maxEpisodeDownload > rs2.rows.length ? rs2.rows.length : maxEpisodeDownload
             for (var j=0; j < loopCount; j++) {
-                if (!rs2.rows.item(j).downloadedfile && !rs2.rows.item(j).listened && rs2.rows.item(j).audiourl) {
-                    podbird.downloadEpisode(rs.rows.item(i).image, rs2.rows.item(j).name, rs2.rows.item(j).guid, rs2.rows.item(j).audiourl, podbird.settings.downloadOverWifiOnly)
-                    tx.executeSql("UPDATE Episode SET queued=1 WHERE guid = ?", [rs2.rows.item(j).guid]);
+                var  episode = rs2.rows.item(j);
+                if ( !episode.downloadedfile && !episode.listened && episode.audiourl && !episode.queued ) {
+                    podbird.downloadEpisode(podcast.image, episode.name, episode.guid, episode.audiourl, podbird.settings.downloadOverWifiOnly)
+                    tx.executeSql("UPDATE Episode SET queued=1 WHERE guid = ?", [episode.guid]);
                 }
             }
         }

=== modified file 'po/podbird.nik90.pot'
--- po/podbird.nik90.pot	2016-10-24 19:08:35 +0000
+++ po/podbird.nik90.pot	2016-11-28 09:51:07 +0000
@@ -8,7 +8,7 @@
 msgstr ""
 "Project-Id-Version: \n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2016-10-24 19:03+0000\n"
+"POT-Creation-Date: 2016-11-28 09:45+0000\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <[e-mail]>\n"