Merge #2756 Cache downloads individually upon completion

CHANGELOG.md

@@ -11,6 +11,7 @@ All notable changes to this project will be documented in this file.
 ### Bugfixes
 
 - [Core] Save registry inside of scan transaction (#2755 by: HebaruSan; reviewed: DasSkelett)
+- [Core] Cache downloads individually upon completion (#2756 by: HebaruSan; reviewed: DasSkelett)
 
 ## v1.26.2
 

Cmdline/Action/Replace.cs

@@ -155,7 +155,7 @@ public int RunCommand(CKAN.KSP ksp, object raw_options)
                 // TODO: These instances all need to go.
                 try
                 {
-                    ModuleInstaller.GetInstance(ksp, manager.Cache, User).Replace(to_replace, replace_ops, new NetAsyncModulesDownloader(User));
+                    ModuleInstaller.GetInstance(ksp, manager.Cache, User).Replace(to_replace, replace_ops, new NetAsyncModulesDownloader(User, manager.Cache));
                     User.RaiseMessage("\r\nDone!\r\n");
                 }
                 catch (DependencyNotSatisfiedKraken ex)

Cmdline/Action/Upgrade.cs

@@ -129,13 +129,13 @@ public int RunCommand(CKAN.KSP ksp, object raw_options)
 
                     }
 
-                    ModuleInstaller.GetInstance(ksp, manager.Cache, User).Upgrade(to_upgrade, new NetAsyncModulesDownloader(User));
+                    ModuleInstaller.GetInstance(ksp, manager.Cache, User).Upgrade(to_upgrade, new NetAsyncModulesDownloader(User, manager.Cache));
                 }
                 else
                 {
                     // TODO: These instances all need to go.
                     Search.AdjustModulesCase(ksp, options.modules);
-                    ModuleInstaller.GetInstance(ksp, manager.Cache, User).Upgrade(options.modules, new NetAsyncModulesDownloader(User));
+                    ModuleInstaller.GetInstance(ksp, manager.Cache, User).Upgrade(options.modules, new NetAsyncModulesDownloader(User, manager.Cache));
                 }
             }
             catch (ModuleNotFoundKraken kraken)

ConsoleUI/InstallScreen.cs

@@ -61,7 +61,7 @@ public override void Run(Action process = null)
                             inst.UninstallList(plan.Remove);
                             plan.Remove.Clear();
                         }
-                        NetAsyncModulesDownloader dl = new NetAsyncModulesDownloader(this);
+                        NetAsyncModulesDownloader dl = new NetAsyncModulesDownloader(this, manager.Cache);
                         if (plan.Upgrade.Count > 0) {
                             inst.Upgrade(plan.Upgrade, dl);
                             plan.Upgrade.Clear();

ConsoleUI/ModInfoScreen.cs

@@ -512,13 +512,13 @@ private string HostedOn()
         private void Download()
         {
             ProgressScreen            ps   = new ProgressScreen($"Downloading {mod.identifier}");
-            NetAsyncModulesDownloader dl   = new NetAsyncModulesDownloader(ps);
+            NetAsyncModulesDownloader dl   = new NetAsyncModulesDownloader(ps, manager.Cache);
             ModuleInstaller           inst = ModuleInstaller.GetInstance(manager.CurrentInstance, manager.Cache, ps);
             LaunchSubScreen(
                 ps,
                 () => {
                     try {
-                        dl.DownloadModules(manager.Cache, new List<CkanModule> {mod});
+                        dl.DownloadModules(new List<CkanModule> {mod});
                         if (!manager.Cache.IsMaybeCachedZip(mod)) {
                             ps.RaiseError("Download failed, file is corrupted");
                         }

Core/ModuleInstaller.cs

@@ -199,10 +199,10 @@ public void InstallList(ICollection<CkanModule> modules, RelationshipResolverOpt
             {
                 if (downloader == null)
                 {
-                    downloader = new NetAsyncModulesDownloader(User);
+                    downloader = new NetAsyncModulesDownloader(User, Cache);
                 }
 
-                downloader.DownloadModules(Cache, downloads);
+                downloader.DownloadModules(downloads);
             }
 
             // We're about to install all our mods; so begin our transaction.
@@ -1179,7 +1179,7 @@ private void DownloadModules(IEnumerable<CkanModule> mods, IDownloader downloade
 
             if (downloads.Count > 0)
             {
-                downloader.DownloadModules(Cache, downloads);
+                downloader.DownloadModules(downloads);
             }
         }
 

Core/Net/IDownloader.cs

@@ -11,7 +11,7 @@ public interface IDownloader
         /// Even if modules share download URLs, they will only be downloaded once.
         /// Blocks until the downloads are complete, cancelled, or errored.
         /// </summary>
-        void DownloadModules(NetModuleCache cache, IEnumerable<CkanModule> modules);
+        void DownloadModules(IEnumerable<CkanModule> modules);
 
         /// <summary>
         /// Cancel any running downloads.

Core/Net/Net.cs

@@ -191,12 +191,15 @@ public static void DownloadWithProgress(ICollection<DownloadTarget> downloadTarg
         {
             new NetAsyncDownloader(user ?? new NullUser())
             {
-                onCompleted = (urls, filenames, errors) =>
+                onOneCompleted = (url, filename, error) =>
                 {
-                    if (filenames == null || urls == null) return;
-                    for (var i = 0; i < Math.Min(urls.Length, filenames.Length); i++)
+                    if (error != null)
                     {
-                        File.Move(filenames[i], downloadTargets.First(p => p.url == urls[i]).filename);
+                        user?.RaiseError(error.ToString());
+                    }
+                    else
+                    {
+                        File.Move(filename, downloadTargets.First(p => p.url == url).filename);
                     }
                 }
             }.DownloadAndWait(downloadTargets);

Core/Net/NetAsyncDownloader.cs

@@ -109,10 +109,8 @@ private void ResetAgent()
         private volatile bool download_canceled;
         private readonly ManualResetEvent complete_or_canceled;
 
-        // Called on completion (including on error)
-        // Called with ALL NULLS on error.
-        public delegate void NetAsyncCompleted(Uri[] urls, string[] filenames, Exception[] errors);
-        public NetAsyncCompleted onCompleted;
+        public delegate void NetAsyncOneCompleted(Uri url, string filename, Exception error);
+        public NetAsyncOneCompleted onOneCompleted;
 
         // When using the curlsharp downloader, this contains all the threads
         // that are working for us.
@@ -399,15 +397,11 @@ public void CancelDownload()
         {
             log.Info("Cancelling download");
             download_canceled = true;
-            triggerCompleted(null, null, null);
+            triggerCompleted();
         }
 
-        private void triggerCompleted(Uri[] file_urls, string[] file_paths, Exception[] errors)
+        private void triggerCompleted()
         {
-            if (onCompleted != null)
-            {
-                onCompleted.Invoke(file_urls, file_paths, errors);
-            }
             // Signal that we're done.
             complete_or_canceled.Set();
         }
@@ -492,41 +486,19 @@ private void FileDownloadComplete(int index, Exception error)
                 }
                 else
                 {
-                    // If there was an error, remember it, but we won't raise it until
-                    // all downloads are finished or cancelled.
                     downloads[index].error = error;
                 }
             }
             else
             {
                 log.InfoFormat("Finished downloading {0}", downloads[index].url);
             }
+            onOneCompleted.Invoke(downloads[index].url, downloads[index].path, downloads[index].error);
 
             if (++completed_downloads >= downloads.Count)
             {
-                FinalizeDownloads();
-            }
-        }
-
-        private void FinalizeDownloads()
-        {
-            log.Info("All files finished downloading");
-
-            Uri[]       fileUrls  = new Uri[downloads.Count];
-            string[]    filePaths = new string[downloads.Count];
-            Exception[] errors    = new Exception[downloads.Count];
-
-            for (int i = 0; i < downloads.Count; ++i)
-            {
-                fileUrls[i]  = downloads[i].url;
-                filePaths[i] = downloads[i].path;
-                errors[i]    = downloads[i].error;
+                triggerCompleted();
             }
-
-            // If we have a callback, then signal that we're done.
-            log.Debug("Signalling completion via callback");
-            triggerCompleted(fileUrls, filePaths, errors);
         }
-
     }
 }

Core/Net/NetAsyncModulesDownloader.cs

@@ -11,32 +11,32 @@ namespace CKAN
     /// </summary>
     public class NetAsyncModulesDownloader : IDownloader
     {
-        public IUser User
+        private IUser User
         {
             get { return downloader.User;  }
-            set { downloader.User = value; }
         }
 
         private static readonly ILog log = LogManager.GetLogger(typeof (NetAsyncModulesDownloader));
 
         private          List<CkanModule>   modules;
         private readonly NetAsyncDownloader downloader;
+        private readonly NetModuleCache     cache;
         private const    string             defaultMimeType = "application/octet-stream";
 
         /// <summary>
         /// Returns a perfectly boring NetAsyncModulesDownloader.
         /// </summary>
-        public NetAsyncModulesDownloader(IUser user)
+        public NetAsyncModulesDownloader(IUser user, NetModuleCache cache)
         {
             modules    = new List<CkanModule>();
             downloader = new NetAsyncDownloader(user);
+            this.cache = cache;
         }
 
-
         /// <summary>
         /// <see cref="IDownloader.DownloadModules(NetFileCache, IEnumerable{CkanModule})"/>
         /// </summary>
-        public void DownloadModules(NetModuleCache cache, IEnumerable<CkanModule> modules)
+        public void DownloadModules(IEnumerable<CkanModule> modules)
         {
             // Walk through all our modules, but only keep the first of each
             // one that has a unique download path.
@@ -46,11 +46,9 @@ public void DownloadModules(NetModuleCache cache, IEnumerable<CkanModule> module
 
             this.modules.AddRange(unique_downloads.Values);
 
-            // Schedule us to process our modules on completion.
-            downloader.onCompleted =
-                (_uris, paths, errors) =>
-                    ModuleDownloadsComplete(cache, _uris, paths, errors);
-
+            // Schedule us to process each module on completion.
+            downloader.onOneCompleted = ModuleDownloadComplete;
+
             try
             {
                 // Start the downloads!
@@ -76,42 +74,28 @@ public void DownloadModules(NetModuleCache cache, IEnumerable<CkanModule> module
             }
         }
 
-        /// <summary>
-        /// Stores all of our files in the cache once done.
-        /// Called by NetAsyncDownloader on completion.
-        /// Called with all nulls on download cancellation.
-        /// </summary>
-        private void ModuleDownloadsComplete(NetModuleCache cache, Uri[] urls, string[] filenames, Exception[] errors)
+        private void ModuleDownloadComplete(Uri url, string filename, Exception error)
         {
-            if (filenames != null)
+            if (error != null)
             {
-                for (int i = 0; i < errors.Length; i++)
+                User.RaiseError(error.ToString());
+            }
+            else
+            {
+                // Cache if this download succeeded
+                try
                 {
-                    if (errors[i] == null)
-                    {
-                        // Cache the downloads that succeeded.
-                        try
-                        {
-                            cache.Store(modules[i], filenames[i], modules[i].StandardName());
-                        }
-                        catch (InvalidModuleFileKraken kraken)
-                        {
-                            User.RaiseError(kraken.ToString());
-                        }
-                        catch (FileNotFoundException e)
-                        {
-                            log.WarnFormat("cache.Store(): FileNotFoundException: {0}", e.Message);
-                        }
-                    }
+                    CkanModule module = modules.First(m => m.download == url);
+                    cache.Store(module, filename, module.StandardName());
+                    File.Delete(filename);
                 }
-
-                // Finally, remove all our temp files.
-                // We probably *could* have used Store's integrated move function above, but if we managed
-                // to somehow get two URLs the same in our download set, that could cause right troubles!
-                foreach (string tmpfile in filenames)
+                catch (InvalidModuleFileKraken kraken)
+                {
+                    User.RaiseError(kraken.ToString());
+                }
+                catch (FileNotFoundException e)
                 {
-                    log.DebugFormat("Cleaning up {0}", tmpfile);
-                    File.Delete(tmpfile);
+                    log.WarnFormat("cache.Store(): FileNotFoundException: {0}", e.Message);
                 }
             }
         }

Core/Net/Repo.cs

@@ -211,7 +211,7 @@ You should reinstall them in order to preserve consistency with the repository.
                     {
                         installer.Upgrade(
                             new[] { changedIdentifier },
-                            new NetAsyncModulesDownloader(new NullUser()),
+                            new NetAsyncModulesDownloader(new NullUser(), cache),
                             enforceConsistency: false
                         );
                     }

GUI/MainInstall.cs

@@ -149,7 +149,7 @@ private void InstallMods(object sender, DoWorkEventArgs e)
             ShowWaitDialog();
             tabController.SetTabLock(true);
 
-            IDownloader downloader = new NetAsyncModulesDownloader(GUI.user);
+            IDownloader downloader = new NetAsyncModulesDownloader(GUI.user, Manager.Cache);
             cancelCallback = () =>
             {
                 downloader.CancelDownload();

GUI/MainModInfo.cs

@@ -447,9 +447,9 @@ private void CacheMod(object sender, DoWorkEventArgs e)
             Main.Instance.ResetProgress();
             Main.Instance.ClearLog();
 
-            NetAsyncModulesDownloader downloader = new NetAsyncModulesDownloader(Main.Instance.currentUser);
+            NetAsyncModulesDownloader downloader = new NetAsyncModulesDownloader(Main.Instance.currentUser, Main.Instance.Manager.Cache);
 
-            downloader.DownloadModules(Main.Instance.Manager.Cache, new List<CkanModule> { (CkanModule)e.Argument });
+            downloader.DownloadModules(new List<CkanModule> { (CkanModule)e.Argument });
             e.Result = e.Argument;
         }
 

Tests/Core/Net/NetAsyncModulesDownloader.cs

@@ -39,7 +39,7 @@ public void Setup()
             CKAN.Repo.Update(registry_manager, ksp.KSP, new NullUser(), TestData.TestKANZip());
 
             // Ready our downloader.
-            async = new CKAN.NetAsyncModulesDownloader(new NullUser());
+            async = new CKAN.NetAsyncModulesDownloader(new NullUser(), manager.Cache);
 
             // General shortcuts
             cache = manager.Cache;
@@ -77,7 +77,7 @@ public void SingleDownload()
             log.InfoFormat("Downloading kOS from {0}", kOS.download);
 
             // Download our module.
-            async.DownloadModules(manager.Cache, modules);
+            async.DownloadModules(modules);
 
             // Assert that we have it, and it passes zip validation.
             Assert.IsTrue(cache.IsCachedZip(kOS));
@@ -100,7 +100,7 @@ public void MultiDownload()
             Assert.IsFalse(cache.IsCachedZip(kOS));
             Assert.IsFalse(cache.IsCachedZip(quick_revert));
 
-            async.DownloadModules(cache, modules);
+            async.DownloadModules(modules);
 
             Assert.IsTrue(cache.IsCachedZip(kOS));
             Assert.IsTrue(cache.IsCachedZip(quick_revert));
@@ -120,7 +120,7 @@ public void RandSdownload()
 
             Assert.IsFalse(cache.IsCachedZip(rAndS), "Module not yet downloaded");
 
-            async.DownloadModules(cache, modules);
+            async.DownloadModules(modules);
 
             Assert.IsTrue(cache.IsCachedZip(rAndS),"Module download successful");
         }

Tests/GUI/GH1866.cs

@@ -70,7 +70,7 @@ public void Up()
             ModuleInstaller.GetInstance(_instance.KSP, _manager.Cache, _manager.User).InstallList(
                 new List<CkanModule> { { _anyVersionModule } },
                 new RelationshipResolverOptions(),
-                new NetAsyncModulesDownloader(_manager.User)
+                new NetAsyncModulesDownloader(_manager.User, _manager.Cache)
             );
 
             // this module is not for "any" version, to provide another to sort against
@@ -139,7 +139,7 @@ public void TestSimple()
                 ModuleInstaller.GetInstance(_instance.KSP, _manager.Cache, _manager.User).InstallList(
                     _modList.ComputeUserChangeSet(null).Select(change => change.Mod.ToCkanModule()).ToList(),
                     new RelationshipResolverOptions(),
-                    new NetAsyncModulesDownloader(_manager.User)
+                    new NetAsyncModulesDownloader(_manager.User, _manager.Cache)
                 );
 
                 // trying to refresh the GUI state will throw a NullReferenceException