From b44a70ff368f228fc27a184e2139d288bada85cc Mon Sep 17 00:00:00 2001 From: Bond-009 Date: Mon, 25 Mar 2019 22:25:32 +0100 Subject: Simplify/remove/clean code * Remove useless runtime check (we only support one) * Remove unused args * Remove a global constant And ofc fix some warnings ;) --- .../Updates/InstallationManager.cs | 32 ++++------------------ 1 file changed, 6 insertions(+), 26 deletions(-) (limited to 'Emby.Server.Implementations/Updates/InstallationManager.cs') diff --git a/Emby.Server.Implementations/Updates/InstallationManager.cs b/Emby.Server.Implementations/Updates/InstallationManager.cs index 301802b8a..7310de55d 100644 --- a/Emby.Server.Implementations/Updates/InstallationManager.cs +++ b/Emby.Server.Implementations/Updates/InstallationManager.cs @@ -12,7 +12,6 @@ using MediaBrowser.Common.Plugins; using MediaBrowser.Common.Progress; using MediaBrowser.Common.Updates; using MediaBrowser.Controller.Configuration; -using MediaBrowser.Model.Cryptography; using MediaBrowser.Model.Events; using MediaBrowser.Model.IO; using MediaBrowser.Model.Serialization; @@ -39,11 +38,10 @@ namespace Emby.Server.Implementations.Updates /// /// The completed installations /// - private ConcurrentBag CompletedInstallationsInternal { get; set; } + private ConcurrentBag _completedInstallationsInternal; - public IEnumerable CompletedInstallations => CompletedInstallationsInternal; + public IEnumerable CompletedInstallations => _completedInstallationsInternal; - #region PluginUninstalled Event /// /// Occurs when [plugin uninstalled]. /// @@ -57,9 +55,7 @@ namespace Emby.Server.Implementations.Updates { PluginUninstalled?.Invoke(this, new GenericEventArgs { Argument = plugin }); } - #endregion - #region PluginUpdated Event /// /// Occurs when [plugin updated]. /// @@ -77,9 +73,7 @@ namespace Emby.Server.Implementations.Updates _applicationHost.NotifyPendingRestart(); } - #endregion - #region PluginInstalled Event /// /// Occurs when [plugin updated]. /// @@ -96,7 +90,6 @@ namespace Emby.Server.Implementations.Updates _applicationHost.NotifyPendingRestart(); } - #endregion /// /// The _logger @@ -115,12 +108,8 @@ namespace Emby.Server.Implementations.Updates /// The application host. private readonly IApplicationHost _applicationHost; - private readonly ICryptoProvider _cryptographyProvider; private readonly IZipClient _zipClient; - // netframework or netcore - private readonly string _packageRuntime; - public InstallationManager( ILoggerFactory loggerFactory, IApplicationHost appHost, @@ -129,9 +118,7 @@ namespace Emby.Server.Implementations.Updates IJsonSerializer jsonSerializer, IServerConfigurationManager config, IFileSystem fileSystem, - ICryptoProvider cryptographyProvider, - IZipClient zipClient, - string packageRuntime) + IZipClient zipClient) { if (loggerFactory == null) { @@ -139,18 +126,16 @@ namespace Emby.Server.Implementations.Updates } CurrentInstallations = new List>(); - CompletedInstallationsInternal = new ConcurrentBag(); + _completedInstallationsInternal = new ConcurrentBag(); + _logger = loggerFactory.CreateLogger(nameof(InstallationManager)); _applicationHost = appHost; _appPaths = appPaths; _httpClient = httpClient; _jsonSerializer = jsonSerializer; _config = config; _fileSystem = fileSystem; - _cryptographyProvider = cryptographyProvider; _zipClient = zipClient; - _packageRuntime = packageRuntime; - _logger = loggerFactory.CreateLogger(nameof(InstallationManager)); } private static Version GetPackageVersion(PackageVersionInfo version) @@ -222,11 +207,6 @@ namespace Emby.Server.Implementations.Updates continue; } - if (string.IsNullOrEmpty(version.runtimes) || version.runtimes.IndexOf(_packageRuntime, StringComparison.OrdinalIgnoreCase) == -1) - { - continue; - } - versions.Add(version); } @@ -448,7 +428,7 @@ namespace Emby.Server.Implementations.Updates CurrentInstallations.Remove(tuple); } - CompletedInstallationsInternal.Add(installationInfo); + _completedInstallationsInternal.Add(installationInfo); PackageInstallationCompleted?.Invoke(this, installationEventArgs); } -- cgit v1.2.3 From 05a4161fd388d7ecda2f1b4a6ec6d02ee7b4488e Mon Sep 17 00:00:00 2001 From: Joshua Boniface Date: Wed, 3 Apr 2019 19:43:02 -0400 Subject: Correct the installation and removal of plugins Upgrading plugins was broken for various reasons. There are four fixes and a minor one: 1. Use a directory name based only on the `Name` of the plugin, not the source filename, which contains the version. Avoids strange duplication of the plugin. 2. Use the new directory name for the deletes if it's present, so that installation and removal happen at that directory level and we don't leave empty folders laying around. Ensures we properly remove additional resources in plugins too, not just the main `.dll` file. 3. Ignore the incoming `target` when installing, and always set it ourself to the proper directory, which would matter when reinstalling. 4. Deletes an existing target directory before installing if it exists. Note that not calling any of the plugin removal code is intentional; I suspect that would delete configurations unexpectedly when upgrading which would be annoying. This way, it just replaces the files and then reloads. 5. (Minor) Added some actual debug messages around the plugin download section so failures can be more accurately seen. --- Emby.Server.Implementations/ApplicationHost.cs | 2 +- .../Updates/InstallationManager.cs | 41 ++++++++++++++++++---- 2 files changed, 36 insertions(+), 7 deletions(-) (limited to 'Emby.Server.Implementations/Updates/InstallationManager.cs') diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs index dbdf246a2..05f8a8a5e 100644 --- a/Emby.Server.Implementations/ApplicationHost.cs +++ b/Emby.Server.Implementations/ApplicationHost.cs @@ -1046,7 +1046,7 @@ namespace Emby.Server.Implementations private async void PluginInstalled(object sender, GenericEventArgs args) { - string dir = Path.Combine(ApplicationPaths.PluginsPath, Path.GetFileNameWithoutExtension(args.Argument.targetFilename)); + string dir = Path.Combine(ApplicationPaths.PluginsPath, args.Argument.name); var types = Directory.EnumerateFiles(dir, "*.dll", SearchOption.AllDirectories) .Select(x => Assembly.LoadFrom(x)) .SelectMany(x => x.ExportedTypes) diff --git a/Emby.Server.Implementations/Updates/InstallationManager.cs b/Emby.Server.Implementations/Updates/InstallationManager.cs index 7310de55d..e5ba813a6 100644 --- a/Emby.Server.Implementations/Updates/InstallationManager.cs +++ b/Emby.Server.Implementations/Updates/InstallationManager.cs @@ -85,9 +85,11 @@ namespace Emby.Server.Implementations.Updates private void OnPluginInstalled(PackageVersionInfo package) { _logger.LogInformation("New plugin installed: {0} {1} {2}", package.name, package.versionStr ?? string.Empty, package.classification); + _logger.LogDebug("{String}", package.name); PluginInstalled?.Invoke(this, new GenericEventArgs { Argument = package }); + _logger.LogDebug("{String}", package.name); _applicationHost.NotifyPendingRestart(); } @@ -518,12 +520,12 @@ namespace Emby.Server.Implementations.Updates return; } - if (target == null) - { - target = Path.Combine(_appPaths.PluginsPath, Path.GetFileNameWithoutExtension(package.targetFilename)); - } + // Always override the passed-in target (which is a file) and figure it out again + target = Path.Combine(_appPaths.PluginsPath, package.name); + _logger.LogDebug("Installing plugin to {Filename}.", target); // Download to temporary file so that, if interrupted, it won't destroy the existing installation + _logger.LogDebug("Downloading ZIP."); var tempFile = await _httpClient.GetTempFile(new HttpRequestOptions { Url = package.sourceUrl, @@ -536,9 +538,17 @@ namespace Emby.Server.Implementations.Updates // TODO: Validate with a checksum, *properly* + // Check if the target directory already exists, and remove it if so + if (Directory.Exists(target)) + { + _logger.LogDebug("Deleting existing plugin at {Filename}.", target); + Directory.Delete(target, true); + } + // Success - move it to the real target try { + _logger.LogDebug("Extracting ZIP {TempFile} to {Filename}.", tempFile, target); using (var stream = File.OpenRead(tempFile)) { _zipClient.ExtractAllFromZip(stream, target, true); @@ -552,6 +562,7 @@ namespace Emby.Server.Implementations.Updates try { + _logger.LogDebug("Deleting temporary file {Filename}.", tempFile); _fileSystem.DeleteFile(tempFile); } catch (IOException ex) @@ -574,7 +585,18 @@ namespace Emby.Server.Implementations.Updates _applicationHost.RemovePlugin(plugin); var path = plugin.AssemblyFilePath; - _logger.LogInformation("Deleting plugin file {0}", path); + bool is_path_directory = false; + // Check if we have a plugin directory we should remove too + if (Path.GetDirectoryName(plugin.AssemblyFilePath) != _appPaths.PluginsPath) + { + path = Path.GetDirectoryName(plugin.AssemblyFilePath); + is_path_directory = true; + _logger.LogInformation("Deleting plugin directory {0}", path); + } + else + { + _logger.LogInformation("Deleting plugin file {0}", path); + } // Make this case-insensitive to account for possible incorrect assembly naming var file = _fileSystem.GetFilePaths(Path.GetDirectoryName(path)) @@ -585,7 +607,14 @@ namespace Emby.Server.Implementations.Updates path = file; } - _fileSystem.DeleteFile(path); + if (is_path_directory) + { + Directory.Delete(path, true); + } + else + { + _fileSystem.DeleteFile(path); + } var list = _config.Configuration.UninstalledPlugins.ToList(); var filename = Path.GetFileName(path); -- cgit v1.2.3 From 09505e0988ec53538c3ecc3d243b1d1a5edac8c6 Mon Sep 17 00:00:00 2001 From: Joshua Boniface Date: Thu, 4 Apr 2019 01:54:31 -0400 Subject: Apply review feedback Remove a few superfluous/testing log statements, and print the deletion debug messages when it occurs rather than earlier. Use a nicer name for the isDirectory variable. --- .../Updates/InstallationManager.cs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) (limited to 'Emby.Server.Implementations/Updates/InstallationManager.cs') diff --git a/Emby.Server.Implementations/Updates/InstallationManager.cs b/Emby.Server.Implementations/Updates/InstallationManager.cs index e5ba813a6..95fbefe00 100644 --- a/Emby.Server.Implementations/Updates/InstallationManager.cs +++ b/Emby.Server.Implementations/Updates/InstallationManager.cs @@ -85,11 +85,9 @@ namespace Emby.Server.Implementations.Updates private void OnPluginInstalled(PackageVersionInfo package) { _logger.LogInformation("New plugin installed: {0} {1} {2}", package.name, package.versionStr ?? string.Empty, package.classification); - _logger.LogDebug("{String}", package.name); PluginInstalled?.Invoke(this, new GenericEventArgs { Argument = package }); - _logger.LogDebug("{String}", package.name); _applicationHost.NotifyPendingRestart(); } @@ -585,17 +583,12 @@ namespace Emby.Server.Implementations.Updates _applicationHost.RemovePlugin(plugin); var path = plugin.AssemblyFilePath; - bool is_path_directory = false; + bool isDirectory = false; // Check if we have a plugin directory we should remove too if (Path.GetDirectoryName(plugin.AssemblyFilePath) != _appPaths.PluginsPath) { path = Path.GetDirectoryName(plugin.AssemblyFilePath); - is_path_directory = true; - _logger.LogInformation("Deleting plugin directory {0}", path); - } - else - { - _logger.LogInformation("Deleting plugin file {0}", path); + isDirectory = true; } // Make this case-insensitive to account for possible incorrect assembly naming @@ -607,12 +600,14 @@ namespace Emby.Server.Implementations.Updates path = file; } - if (is_path_directory) + if (isDirectory) { + _logger.LogInformation("Deleting plugin directory {0}", path); Directory.Delete(path, true); } else { + _logger.LogInformation("Deleting plugin file {0}", path); _fileSystem.DeleteFile(path); } -- cgit v1.2.3 From 754e76a61bbf4bbda5a4a1073737c07bf28f5f3a Mon Sep 17 00:00:00 2001 From: Joshua Boniface Date: Thu, 4 Apr 2019 02:34:23 -0400 Subject: Add TODO to remove string target --- Emby.Server.Implementations/Updates/InstallationManager.cs | 2 ++ 1 file changed, 2 insertions(+) (limited to 'Emby.Server.Implementations/Updates/InstallationManager.cs') diff --git a/Emby.Server.Implementations/Updates/InstallationManager.cs b/Emby.Server.Implementations/Updates/InstallationManager.cs index 95fbefe00..6833c20c3 100644 --- a/Emby.Server.Implementations/Updates/InstallationManager.cs +++ b/Emby.Server.Implementations/Updates/InstallationManager.cs @@ -509,6 +509,8 @@ namespace Emby.Server.Implementations.Updates private async Task PerformPackageInstallation(IProgress progress, string target, PackageVersionInfo package, CancellationToken cancellationToken) { + // TODO: Remove the `string target` argument as it is not used any longer + var extension = Path.GetExtension(package.targetFilename); var isArchive = string.Equals(extension, ".zip", StringComparison.OrdinalIgnoreCase); -- cgit v1.2.3