Merge pull request #17820 from Shadowghost/only-download-missing-plugin-images

Only download missing plugin images

2 files changed, 87 insertions, 22 deletions

diff --git a/Emby.Server.Implementations/Plugins/PluginManager.cs b/Emby.Server.Implementations/Plugins/PluginManager.cs
index 8d29d6a512..f7ad9b9498 100644
--- a/Emby.Server.Implementations/Plugins/PluginManager.cs
+++ b/Emby.Server.Implementations/Plugins/PluginManager.cs
@@ -395,29 +395,11 @@ namespace Emby.Server.Implementations.Plugins
                 var url = new Uri(packageInfo.ImageUrl);
                 imagePath = Path.Join(path, url.Segments[^1]);
 
-                var fileStream = AsyncFile.OpenWrite(imagePath);
-                Stream? downloadStream = null;
-                try
+                // The catalog is refreshed on every dashboard visit and rewrites the manifest of
+                // every installed plugin, so only fetch an image that is actually missing.
+                if (!ImageExists(imagePath))
                 {
-                    downloadStream = await HttpClientFactory
-                        .CreateClient(NamedClient.Default)
-                        .GetStreamAsync(url)
-                        .ConfigureAwait(false);
-
-                    await downloadStream.CopyToAsync(fileStream).ConfigureAwait(false);
-                }
-                catch (HttpRequestException ex)
-                {
-                    _logger.LogError(ex, "Failed to download image to path {Path} on disk.", imagePath);
-                    imagePath = string.Empty;
-                }
-                finally
-                {
-                    await fileStream.DisposeAsync().ConfigureAwait(false);
-                    if (downloadStream is not null)
-                    {
-                        await downloadStream.DisposeAsync().ConfigureAwait(false);
-                    }
+                    imagePath = await DownloadImage(url, imagePath).ConfigureAwait(false);
                 }
             }
 
@@ -456,6 +438,67 @@ namespace Emby.Server.Implementations.Plugins
             }
         }
 
+        private static bool ImageExists(string imagePath)
+        {
+            var image = new FileInfo(imagePath);
+
+            // A previous download may have been interrupted, leaving an empty file behind.
+            return image.Exists && image.Length > 0;
+        }
+
+        private async Task<string> DownloadImage(Uri url, string imagePath)
+        {
+            // Download to a temporary file and move it into place, so that neither a failed download
+            // nor a concurrent one can be observed as a partially written image.
+            var tempPath = imagePath + "." + Path.GetRandomFileName();
+
+            try
+            {
+                var fileStream = AsyncFile.Create(tempPath);
+                Stream? downloadStream = null;
+                try
+                {
+                    downloadStream = await HttpClientFactory
+                        .CreateClient(NamedClient.Default)
+                        .GetStreamAsync(url)
+                        .ConfigureAwait(false);
+
+                    await downloadStream.CopyToAsync(fileStream).ConfigureAwait(false);
+                }
+                finally
+                {
+                    await fileStream.DisposeAsync().ConfigureAwait(false);
+                    if (downloadStream is not null)
+                    {
+                        await downloadStream.DisposeAsync().ConfigureAwait(false);
+                    }
+                }
+
+                File.Move(tempPath, imagePath, true);
+
+                return imagePath;
+            }
+            catch (Exception ex) when (ex is HttpRequestException or IOException or UnauthorizedAccessException)
+            {
+                _logger.LogError(ex, "Failed to download image to path {Path} on disk.", imagePath);
+                TryDeleteFile(tempPath);
+
+                return string.Empty;
+            }
+        }
+
+        private void TryDeleteFile(string path)
+        {
+            try
+            {
+                File.Delete(path);
+            }
+            catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
+            {
+                _logger.LogWarning(ex, "Unable to delete {Path}.", path);
+            }
+        }
+
         /// <summary>
         /// Reconciles the manifest against any properties that exist locally in a pre-packaged meta.json found at the path.
         /// If no file is found, no reconciliation occurs.
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Plugins/PluginManagerTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Plugins/PluginManagerTests.cs
index 265b6a7f43..ee41b968e1 100644
--- a/tests/Jellyfin.Server.Implementations.Tests/Plugins/PluginManagerTests.cs
+++ b/tests/Jellyfin.Server.Implementations.Tests/Plugins/PluginManagerTests.cs
@@ -240,6 +240,28 @@ namespace Jellyfin.Server.Implementations.Tests.Plugins
         }
 
         [Fact]
+        public async Task PopulateManifest_ExistingImage_IsNotDownloaded()
+        {
+            const string ImageContent = "not really a png";
+
+            var packageInfo = GenerateTestPackage();
+            packageInfo.ImageUrl = "https://example.org/some-plugin.png";
+
+            var imagePath = Path.Combine(_pluginPath, "some-plugin.png");
+            await File.WriteAllTextAsync(imagePath, ImageContent, TestContext.Current.CancellationToken);
+
+            // The application host is null, so attempting to download the image would throw.
+            var pluginManager = new PluginManager(new NullLogger<PluginManager>(), null!, null!, null!, new Version(1, 0));
+
+            Assert.True(await pluginManager.PopulateManifest(packageInfo, new Version(1, 0), _pluginPath, PluginStatus.Active));
+
+            var result = pluginManager.LoadManifest(_pluginPath).Manifest;
+
+            Assert.Equal(imagePath, result.ImagePath);
+            Assert.Equal(ImageContent, await File.ReadAllTextAsync(imagePath, TestContext.Current.CancellationToken));
+        }
+
+        [Fact]
         public async Task PopulateManifest_ExistingMetafileMismatchedIds_Status_Malfunctioned()
         {
             var packageInfo = GenerateTestPackage();