Merge pull request #1462 from Bond-009/installationmanager

Improvements to InstallationManager

Emby.Server.Implementations/Activity/ActivityLogEntryPoint.cs

@@ -346,7 +346,7 @@ namespace Emby.Server.Implementations.Activity
             });
         }
 
-        private void OnPluginUpdated(object sender, GenericEventArgs<Tuple<IPlugin, PackageVersionInfo>> e)
+        private void OnPluginUpdated(object sender, GenericEventArgs<(IPlugin, PackageVersionInfo)> e)
         {
             CreateLogEntry(new ActivityLogEntry
             {

Emby.Server.Implementations/Emby.Server.Implementations.csproj

@@ -47,6 +47,11 @@
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
   </PropertyGroup>
 
+  <PropertyGroup>
+    <!-- We need C# 7.3 to compare tuples-->
+    <LangVersion>latest</LangVersion>
+  </PropertyGroup>
+
   <PropertyGroup Condition=" '$(Configuration)' == 'Release' ">
     <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
   </PropertyGroup>

Emby.Server.Implementations/ScheduledTasks/Tasks/PluginUpdateTask.cs

@@ -1,4 +1,3 @@
-using MediaBrowser.Common;
 using MediaBrowser.Common.Updates;
 using MediaBrowser.Model.Net;
 using System;
@@ -25,13 +24,10 @@ namespace Emby.Server.Implementations.ScheduledTasks
 
         private readonly IInstallationManager _installationManager;
 
-        private readonly IApplicationHost _appHost;
-
-        public PluginUpdateTask(ILogger logger, IInstallationManager installationManager, IApplicationHost appHost)
+        public PluginUpdateTask(ILogger logger, IInstallationManager installationManager)
         {
             _logger = logger;
             _installationManager = installationManager;
-            _appHost = appHost;
         }
 
         /// <summary>
@@ -40,14 +36,11 @@ namespace Emby.Server.Implementations.ScheduledTasks
         /// <returns>IEnumerable{BaseTaskTrigger}.</returns>
         public IEnumerable<TaskTriggerInfo> GetDefaultTriggers()
         {
-            return new[] { 
-            
-                // At startup
-                new TaskTriggerInfo {Type = TaskTriggerInfo.TriggerStartup},
-
-                // Every so often
-                new TaskTriggerInfo { Type = TaskTriggerInfo.TriggerInterval, IntervalTicks = TimeSpan.FromHours(24).Ticks}
-            };
+            // At startup
+            yield return new TaskTriggerInfo { Type = TaskTriggerInfo.TriggerStartup };
+
+            // Every so often
+            yield return new TaskTriggerInfo { Type = TaskTriggerInfo.TriggerInterval, IntervalTicks = TimeSpan.FromHours(24).Ticks };
         }
 
         /// <summary>
@@ -72,7 +65,7 @@ namespace Emby.Server.Implementations.ScheduledTasks
 
                 try
                 {
-                    await _installationManager.InstallPackage(package, true, new SimpleProgress<double>(), cancellationToken).ConfigureAwait(false);
+                    await _installationManager.InstallPackage(package, new SimpleProgress<double>(), cancellationToken).ConfigureAwait(false);
                 }
                 catch (OperationCanceledException)
                 {

Emby.Server.Implementations/Updates/InstallationManager.cs

@@ -3,6 +3,7 @@ using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
+using System.Net.Http;
 using System.Threading;
 using System.Threading.Tasks;
 using MediaBrowser.Common;
@@ -33,7 +34,7 @@ namespace Emby.Server.Implementations.Updates
         /// <summary>
         /// The current installations
         /// </summary>
-        public List<Tuple<InstallationInfo, CancellationTokenSource>> CurrentInstallations { get; set; }
+        private List<(InstallationInfo info, CancellationTokenSource token)> _currentInstallations { get; set; }
 
         /// <summary>
         /// The completed installations
@@ -47,49 +48,15 @@ namespace Emby.Server.Implementations.Updates
         /// </summary>
         public event EventHandler<GenericEventArgs<IPlugin>> PluginUninstalled;
 
-        /// <summary>
-        /// Called when [plugin uninstalled].
-        /// </summary>
-        /// <param name="plugin">The plugin.</param>
-        private void OnPluginUninstalled(IPlugin plugin)
-        {
-            PluginUninstalled?.Invoke(this, new GenericEventArgs<IPlugin> { Argument = plugin });
-        }
-
         /// <summary>
         /// Occurs when [plugin updated].
         /// </summary>
-        public event EventHandler<GenericEventArgs<Tuple<IPlugin, PackageVersionInfo>>> PluginUpdated;
-        /// <summary>
-        /// Called when [plugin updated].
-        /// </summary>
-        /// <param name="plugin">The plugin.</param>
-        /// <param name="newVersion">The new version.</param>
-        private void OnPluginUpdated(IPlugin plugin, PackageVersionInfo newVersion)
-        {
-            _logger.LogInformation("Plugin updated: {0} {1} {2}", newVersion.name, newVersion.versionStr ?? string.Empty, newVersion.classification);
-
-            PluginUpdated?.Invoke(this, new GenericEventArgs<Tuple<IPlugin, PackageVersionInfo>> { Argument = new Tuple<IPlugin, PackageVersionInfo>(plugin, newVersion) });
-
-            _applicationHost.NotifyPendingRestart();
-        }
+        public event EventHandler<GenericEventArgs<(IPlugin, PackageVersionInfo)>> PluginUpdated;
 
         /// <summary>
         /// Occurs when [plugin updated].
         /// </summary>
         public event EventHandler<GenericEventArgs<PackageVersionInfo>> PluginInstalled;
-        /// <summary>
-        /// Called when [plugin installed].
-        /// </summary>
-        /// <param name="package">The package.</param>
-        private void OnPluginInstalled(PackageVersionInfo package)
-        {
-            _logger.LogInformation("New plugin installed: {0} {1} {2}", package.name, package.versionStr ?? string.Empty, package.classification);
-
-            PluginInstalled?.Invoke(this, new GenericEventArgs<PackageVersionInfo> { Argument = package });
-
-            _applicationHost.NotifyPendingRestart();
-        }
 
         /// <summary>
         /// The _logger
@@ -111,7 +78,7 @@ namespace Emby.Server.Implementations.Updates
         private readonly IZipClient _zipClient;
 
         public InstallationManager(
-            ILoggerFactory loggerFactory,
+            ILogger<InstallationManager> logger,
             IApplicationHost appHost,
             IApplicationPaths appPaths,
             IHttpClient httpClient,
@@ -120,15 +87,15 @@ namespace Emby.Server.Implementations.Updates
             IFileSystem fileSystem,
             IZipClient zipClient)
         {
-            if (loggerFactory == null)
+            if (logger == null)
             {
-                throw new ArgumentNullException(nameof(loggerFactory));
+                throw new ArgumentNullException(nameof(logger));
             }
 
-            CurrentInstallations = new List<Tuple<InstallationInfo, CancellationTokenSource>>();
+            _currentInstallations = new List<(InstallationInfo, CancellationTokenSource)>();
             _completedInstallationsInternal = new ConcurrentBag<InstallationInfo>();
 
-            _logger = loggerFactory.CreateLogger(nameof(InstallationManager));
+            _logger = logger;
             _applicationHost = appHost;
             _appPaths = appPaths;
             _httpClient = httpClient;
@@ -138,21 +105,12 @@ namespace Emby.Server.Implementations.Updates
             _zipClient = zipClient;
         }
 
-        private static Version GetPackageVersion(PackageVersionInfo version)
-        {
-            return new Version(ValueOrDefault(version.versionStr, "0.0.0.1"));
-        }
-
-        private static string ValueOrDefault(string str, string def)
-        {
-            return string.IsNullOrEmpty(str) ? def : str;
-        }
-
         /// <summary>
         /// Gets all available packages.
         /// </summary>
         /// <returns>Task{List{PackageInfo}}.</returns>
-        public async Task<List<PackageInfo>> GetAvailablePackages(CancellationToken cancellationToken,
+        public async Task<List<PackageInfo>> GetAvailablePackages(
+            CancellationToken cancellationToken,
             bool withRegistration = true,
             string packageType = null,
             Version applicationVersion = null)
@@ -172,22 +130,14 @@ namespace Emby.Server.Implementations.Updates
             {
                 Url = "https://repo.jellyfin.org/releases/plugin/manifest.json",
                 CancellationToken = cancellationToken,
-                Progress = new SimpleProgress<double>(),
                 CacheLength = GetCacheLength()
-            }, "GET").ConfigureAwait(false))
+            }, HttpMethod.Get).ConfigureAwait(false))
+            using (var stream = response.Content)
             {
-                using (var stream = response.Content)
-                {
-                    return FilterPackages(await _jsonSerializer.DeserializeFromStreamAsync<PackageInfo[]>(stream).ConfigureAwait(false));
-                }
+                return FilterPackages(await _jsonSerializer.DeserializeFromStreamAsync<PackageInfo[]>(stream).ConfigureAwait(false));
             }
         }
 
-        private PackageVersionClass GetSystemUpdateLevel()
-        {
-            return _applicationHost.SystemUpdateLevel;
-        }
-
         private static TimeSpan GetCacheLength()
         {
             return TimeSpan.FromMinutes(3);
@@ -211,7 +161,7 @@ namespace Emby.Server.Implementations.Updates
                 }
 
                 package.versions = versions
-                    .OrderByDescending(GetPackageVersion)
+                    .OrderByDescending(x => x.Version)
                     .ToArray();
 
                 if (package.versions.Length == 0)
@@ -294,7 +244,7 @@ namespace Emby.Server.Implementations.Updates
                 return null;
             }
 
-            return package.versions.FirstOrDefault(v => GetPackageVersion(v).Equals(version) && v.classification == classification);
+            return package.versions.FirstOrDefault(v => v.Version == version && v.classification == classification);
         }
 
         /// <summary>
@@ -331,7 +281,7 @@ namespace Emby.Server.Implementations.Updates
             }
 
             return package.versions
-                .OrderByDescending(GetPackageVersion)
+                .OrderByDescending(x => x.Version)
                 .FirstOrDefault(v => v.classification <= classification && IsPackageVersionUpToDate(v, currentServerVersion));
         }
 
@@ -346,14 +296,14 @@ namespace Emby.Server.Implementations.Updates
         {
             var catalog = await GetAvailablePackagesWithoutRegistrationInfo(cancellationToken).ConfigureAwait(false);
 
-            var systemUpdateLevel = GetSystemUpdateLevel();
+            var systemUpdateLevel = _applicationHost.SystemUpdateLevel;
 
             // Figure out what needs to be installed
             return _applicationHost.Plugins.Select(p =>
             {
                 var latestPluginInfo = GetLatestCompatibleVersion(catalog, p.Name, p.Id.ToString(), applicationVersion, systemUpdateLevel);
 
-                return latestPluginInfo != null && GetPackageVersion(latestPluginInfo) > p.Version ? latestPluginInfo : null;
+                return latestPluginInfo != null && latestPluginInfo.Version > p.Version ? latestPluginInfo : null;
 
             }).Where(i => i != null)
             .Where(p => !string.IsNullOrEmpty(p.sourceUrl) && !CompletedInstallations.Any(i => string.Equals(i.AssemblyGuid, p.guid, StringComparison.OrdinalIgnoreCase)));
@@ -368,7 +318,7 @@ namespace Emby.Server.Implementations.Updates
         /// <param name="cancellationToken">The cancellation token.</param>
         /// <returns>Task.</returns>
         /// <exception cref="ArgumentNullException">package</exception>
-        public async Task InstallPackage(PackageVersionInfo package, bool isPlugin, IProgress<double> progress, CancellationToken cancellationToken)
+        public async Task InstallPackage(PackageVersionInfo package, IProgress<double> progress, CancellationToken cancellationToken)
         {
             if (package == null)
             {
@@ -391,12 +341,12 @@ namespace Emby.Server.Implementations.Updates
 
             var innerCancellationTokenSource = new CancellationTokenSource();
 
-            var tuple = new Tuple<InstallationInfo, CancellationTokenSource>(installationInfo, innerCancellationTokenSource);
+            var tuple = (installationInfo, innerCancellationTokenSource);
 
             // Add it to the in-progress list
-            lock (CurrentInstallations)
+            lock (_currentInstallations)
             {
-                CurrentInstallations.Add(tuple);
+                _currentInstallations.Add(tuple);
             }
 
             var innerProgress = new ActionableProgress<double>();
@@ -421,11 +371,11 @@ namespace Emby.Server.Implementations.Updates
 
             try
             {
-                await InstallPackageInternal(package, isPlugin, innerProgress, linkedToken).ConfigureAwait(false);
+                await InstallPackageInternal(package, innerProgress, linkedToken).ConfigureAwait(false);
 
-                lock (CurrentInstallations)
+                lock (_currentInstallations)
                 {
-                    CurrentInstallations.Remove(tuple);
+                    _currentInstallations.Remove(tuple);
                 }
 
                 _completedInstallationsInternal.Add(installationInfo);
@@ -434,9 +384,9 @@ namespace Emby.Server.Implementations.Updates
             }
             catch (OperationCanceledException)
             {
-                lock (CurrentInstallations)
+                lock (_currentInstallations)
                 {
-                    CurrentInstallations.Remove(tuple);
+                    _currentInstallations.Remove(tuple);
                 }
 
                 _logger.LogInformation("Package installation cancelled: {0} {1}", package.name, package.versionStr);
@@ -449,9 +399,9 @@ namespace Emby.Server.Implementations.Updates
             {
                 _logger.LogError(ex, "Package installation failed");
 
-                lock (CurrentInstallations)
+                lock (_currentInstallations)
                 {
-                    CurrentInstallations.Remove(tuple);
+                    _currentInstallations.Remove(tuple);
                 }
 
                 PackageInstallationFailed?.Invoke(this, new InstallationFailedEventArgs
@@ -477,16 +427,11 @@ namespace Emby.Server.Implementations.Updates
         /// <param name="progress">The progress.</param>
         /// <param name="cancellationToken">The cancellation token.</param>
         /// <returns>Task.</returns>
-        private async Task InstallPackageInternal(PackageVersionInfo package, bool isPlugin, IProgress<double> progress, CancellationToken cancellationToken)
+        private async Task InstallPackageInternal(PackageVersionInfo package, IProgress<double> progress, CancellationToken cancellationToken)
         {
-            IPlugin plugin = null;
-
-            if (isPlugin)
-            {
-                // Set last update time if we were installed before
-                plugin = _applicationHost.Plugins.FirstOrDefault(p => string.Equals(p.Id.ToString(), package.guid, StringComparison.OrdinalIgnoreCase))
+            // Set last update time if we were installed before
+            IPlugin plugin = _applicationHost.Plugins.FirstOrDefault(p => string.Equals(p.Id.ToString(), package.guid, StringComparison.OrdinalIgnoreCase))
                            ?? _applicationHost.Plugins.FirstOrDefault(p => p.Name.Equals(package.name, StringComparison.OrdinalIgnoreCase));
-            }
 
             string targetPath = plugin == null ? null : plugin.AssemblyFilePath;
 
@@ -494,17 +439,20 @@ namespace Emby.Server.Implementations.Updates
             await PerformPackageInstallation(progress, targetPath, package, cancellationToken).ConfigureAwait(false);
 
             // Do plugin-specific processing
-            if (isPlugin)
+            if (plugin == null)
             {
-                if (plugin == null)
-                {
-                    OnPluginInstalled(package);
-                }
-                else
-                {
-                    OnPluginUpdated(plugin, package);
-                }
+                _logger.LogInformation("New plugin installed: {0} {1} {2}", package.name, package.versionStr ?? string.Empty, package.classification);
+
+                PluginInstalled?.Invoke(this, new GenericEventArgs<PackageVersionInfo>(package));
             }
+            else
+            {
+                _logger.LogInformation("Plugin updated: {0} {1} {2}", package.name, package.versionStr ?? string.Empty, package.classification);
+
+                PluginUpdated?.Invoke(this, new GenericEventArgs<(IPlugin, PackageVersionInfo)>((plugin, package)));
+            }
+
+            _applicationHost.NotifyPendingRestart();
         }
 
         private async Task PerformPackageInstallation(IProgress<double> progress, string target, PackageVersionInfo package, CancellationToken cancellationToken)
@@ -622,11 +570,34 @@ namespace Emby.Server.Implementations.Updates
                 _config.SaveConfiguration();
             }
 
-            OnPluginUninstalled(plugin);
+            PluginUninstalled?.Invoke(this, new GenericEventArgs<IPlugin> { Argument = plugin });
 
             _applicationHost.NotifyPendingRestart();
         }
 
+        /// <inheritdoc/>
+        public bool CancelInstallation(Guid id)
+        {
+            lock (_currentInstallations)
+            {
+                var install = _currentInstallations.Find(x => x.Item1.Id == id);
+                if (install == default((InstallationInfo, CancellationTokenSource)))
+                {
+                    return false;
+                }
+
+                install.Item2.Cancel();
+                _currentInstallations.Remove(install);
+                return true;
+            }
+        }
+
+        public void Dispose()
+        {
+            Dispose(true);
+            GC.SuppressFinalize(this);
+        }
+
         /// <summary>
         /// Releases unmanaged and - optionally - managed resources.
         /// </summary>
@@ -635,21 +606,16 @@ namespace Emby.Server.Implementations.Updates
         {
             if (dispose)
             {
-                lock (CurrentInstallations)
+                lock (_currentInstallations)
                 {
-                    foreach (var tuple in CurrentInstallations)
+                    foreach (var tuple in _currentInstallations)
                     {
                         tuple.Item2.Dispose();
                     }
 
-                    CurrentInstallations.Clear();
+                    _currentInstallations.Clear();
                 }
             }
         }
-
-        public void Dispose()
-        {
-            Dispose(true);
-        }
     }
 }

MediaBrowser.Api/PackageService.cs

@@ -197,7 +197,7 @@ namespace MediaBrowser.Api
                 throw new ResourceNotFoundException(string.Format("Package not found: {0}", request.Name));
             }
 
-            await _installationManager.InstallPackage(package, true, new SimpleProgress<double>(), CancellationToken.None);
+            await _installationManager.InstallPackage(package, new SimpleProgress<double>(), CancellationToken.None);
         }
 
         /// <summary>
@@ -206,13 +206,7 @@ namespace MediaBrowser.Api
         /// <param name="request">The request.</param>
         public void Delete(CancelPackageInstallation request)
         {
-            var info = _installationManager.CurrentInstallations.FirstOrDefault(i => i.Item1.Id.Equals(request.Id));
-
-            if (info != null)
-            {
-                info.Item2.Cancel();
-            }
+            _installationManager.CancelInstallation(new Guid(request.Id));
         }
     }
-
 }

MediaBrowser.Common/Updates/IInstallationManager.cs

@@ -15,11 +15,6 @@ namespace MediaBrowser.Common.Updates
         event EventHandler<InstallationFailedEventArgs> PackageInstallationFailed;
         event EventHandler<InstallationEventArgs> PackageInstallationCancelled;
 
-        /// <summary>
-        /// The current installations
-        /// </summary>
-        List<Tuple<InstallationInfo, CancellationTokenSource>> CurrentInstallations { get; set; }
-
         /// <summary>
         /// The completed installations
         /// </summary>
@@ -33,7 +28,7 @@ namespace MediaBrowser.Common.Updates
         /// <summary>
         /// Occurs when [plugin updated].
         /// </summary>
-        event EventHandler<GenericEventArgs<Tuple<IPlugin, PackageVersionInfo>>> PluginUpdated;
+        event EventHandler<GenericEventArgs<(IPlugin, PackageVersionInfo)>> PluginUpdated;
 
         /// <summary>
         /// Occurs when [plugin updated].
@@ -107,7 +102,7 @@ namespace MediaBrowser.Common.Updates
         /// <param name="cancellationToken">The cancellation token.</param>
         /// <returns>Task.</returns>
         /// <exception cref="ArgumentNullException">package</exception>
-        Task InstallPackage(PackageVersionInfo package, bool isPlugin, IProgress<double> progress, CancellationToken cancellationToken);
+        Task InstallPackage(PackageVersionInfo package, IProgress<double> progress, CancellationToken cancellationToken);
 
         /// <summary>
         /// Uninstalls a plugin
@@ -115,5 +110,12 @@ namespace MediaBrowser.Common.Updates
         /// <param name="plugin">The plugin.</param>
         /// <exception cref="ArgumentException"></exception>
         void UninstallPlugin(IPlugin plugin);
+
+        /// <summary>
+        /// Cancels the installation
+        /// </summary>
+        /// <param name="id">The id of the package that is being installed</param>
+        /// <returns>Returns true if the install was cancelled</returns>
+        bool CancelInstallation(Guid id);
     }
 }

MediaBrowser.Model/Updates/PackageVersionInfo.cs

@@ -30,23 +30,25 @@ namespace MediaBrowser.Model.Updates
         /// The _version
         /// </summary>
         private Version _version;
+
         /// <summary>
         /// Gets or sets the version.
         /// Had to make this an interpreted property since Protobuf can't handle Version
         /// </summary>
         /// <value>The version.</value>
         [IgnoreDataMember]
-        public Version version => _version ?? (_version = new Version(ValueOrDefault(versionStr, "0.0.0.1")));
-
-        /// <summary>
-        /// Values the or default.
-        /// </summary>
-        /// <param name="str">The STR.</param>
-        /// <param name="def">The def.</param>
-        /// <returns>System.String.</returns>
-        private static string ValueOrDefault(string str, string def)
+        public Version Version
         {
-            return string.IsNullOrEmpty(str) ? def : str;
+            get
+            {
+                if (_version == null)
+                {
+                    var ver = versionStr;
+                    _version = new Version(string.IsNullOrEmpty(ver) ? "0.0.0.1" : ver);
+                }
+
+                return _version;
+            }
         }
 
         /// <summary>

MediaBrowser.Providers/Omdb/OmdbProvider.cs

@@ -3,6 +3,7 @@ using System.Collections.Generic;
 using System.Globalization;
 using System.IO;
 using System.Linq;
+using System.Net.Http;
 using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
@@ -347,7 +348,7 @@ namespace MediaBrowser.Providers.Omdb
                 CancellationToken = cancellationToken,
                 BufferContent = true,
                 EnableDefaultUserAgent = true
-            }, "GET");
+            }, HttpMethod.Get);
         }
 
         internal string GetDataFilePath(string imdbId)