From baea9691cc266c7b45f170d7b7be6f27ef80fb55 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Tue, 29 Jan 2019 18:34:10 +0900
Subject: [PATCH] Fix baetmap downloads using too much memory

---
 osu.Game/Beatmaps/BeatmapManager.cs       |  8 +---
 osu.Game/Database/ArchiveModelManager.cs  | 46 ++++++++++++++---------
 osu.Game/Online/API/APIDownloadRequest.cs |  9 +++--
 3 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs
index 42048692fc..21739f16c2 100644
--- a/osu.Game/Beatmaps/BeatmapManager.cs
+++ b/osu.Game/Beatmaps/BeatmapManager.cs
@@ -163,18 +163,14 @@ namespace osu.Game.Beatmaps
                 downloadNotification.Progress = progress;
             };
 
-            request.Success += data =>
+            request.Success += filename =>
             {
                 downloadNotification.Text = $"Importing {beatmapSetInfo.Metadata.Artist} - {beatmapSetInfo.Metadata.Title}";
 
                 Task.Factory.StartNew(() =>
                 {
-                    BeatmapSetInfo importedBeatmap;
-
                     // This gets scheduled back to the update thread, but we want the import to run in the background.
-                    using (var stream = new MemoryStream(data))
-                    using (var archive = new ZipArchiveReader(stream, beatmapSetInfo.ToString()))
-                        importedBeatmap = Import(archive);
+                    var importedBeatmap = Import(filename);
 
                     downloadNotification.CompletionClickAction = () =>
                     {
diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs
index 4b6662178f..e0ebbe209d 100644
--- a/osu.Game/Database/ArchiveModelManager.cs
+++ b/osu.Game/Database/ArchiveModelManager.cs
@@ -150,25 +150,9 @@ namespace osu.Game.Database
                 {
                     notification.Text = $"Importing ({++current} of {paths.Length})\n{Path.GetFileName(path)}";
 
-                    TModel import;
-                    using (ArchiveReader reader = getReaderFrom(path))
-                        imported.Add(import = Import(reader));
+                    imported.Add(Import(path));
 
                     notification.Progress = (float)current / paths.Length;
-
-                    // We may or may not want to delete the file depending on where it is stored.
-                    //  e.g. reconstructing/repairing database with items from default storage.
-                    // Also, not always a single file, i.e. for LegacyFilesystemReader
-                    // TODO: Add a check to prevent files from storage to be deleted.
-                    try
-                    {
-                        if (import != null && File.Exists(path))
-                            File.Delete(path);
-                    }
-                    catch (Exception e)
-                    {
-                        Logger.Error(e, $@"Could not delete original file after import ({Path.GetFileName(path)})");
-                    }
                 }
                 catch (Exception e)
                 {
@@ -195,6 +179,34 @@ namespace osu.Game.Database
             }
         }
 
+        /// <summary>
+        /// Import one <see cref="TModel"/> from the filesystem and delete the file on success.
+        /// </summary>
+        /// <param name="paths">The archive location on disk.</param>
+        /// <returns>The imported model, if successful.</returns>
+        public TModel Import(string path)
+        {
+            TModel import;
+            using (ArchiveReader reader = getReaderFrom(path))
+                import = Import(reader);
+
+            // We may or may not want to delete the file depending on where it is stored.
+            //  e.g. reconstructing/repairing database with items from default storage.
+            // Also, not always a single file, i.e. for LegacyFilesystemReader
+            // TODO: Add a check to prevent files from storage to be deleted.
+            try
+            {
+                if (import != null && File.Exists(path))
+                    File.Delete(path);
+            }
+            catch (Exception e)
+            {
+                Logger.Error(e, $@"Could not delete original file after import ({Path.GetFileName(path)})");
+            }
+
+            return import;
+        }
+
         protected virtual void PresentCompletedImport(IEnumerable<TModel> imported)
         {
         }
diff --git a/osu.Game/Online/API/APIDownloadRequest.cs b/osu.Game/Online/API/APIDownloadRequest.cs
index b9449b57f2..97b869bccd 100644
--- a/osu.Game/Online/API/APIDownloadRequest.cs
+++ b/osu.Game/Online/API/APIDownloadRequest.cs
@@ -1,15 +1,18 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+using System.IO;
 using osu.Framework.IO.Network;
 
 namespace osu.Game.Online.API
 {
     public abstract class APIDownloadRequest : APIRequest
     {
+        private string filename;
+
         protected override WebRequest CreateWebRequest()
         {
-            var request = new WebRequest(Uri);
+            var request = new FileWebRequest(filename = Path.GetTempFileName(), Uri);
             request.DownloadProgress += request_Progress;
             return request;
         }
@@ -23,11 +26,11 @@ namespace osu.Game.Online.API
 
         private void onSuccess()
         {
-            Success?.Invoke(WebRequest.ResponseData);
+            Success?.Invoke(filename);
         }
 
         public event APIProgressHandler Progress;
 
-        public new event APISuccessHandler<byte[]> Success;
+        public new event APISuccessHandler<string> Success;
     }
 }