Lazily load extensions when needed (#623)

This change moves the extension loading into a child scope. This allows
for things like logging to be available while we are loading extensions.

src/cli/Microsoft.DotNet.UpgradeAssistant.Cli/ConsoleRunner.cs

@@ -5,6 +5,9 @@ using System;
 using System.Collections.Generic;
 using System.Threading;
 using System.Threading.Tasks;
+using Autofac;
+using Autofac.Extensions.DependencyInjection;
+using Microsoft.DotNet.UpgradeAssistant.Extensions;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Hosting;
 using Microsoft.Extensions.Logging;
@@ -40,8 +43,19 @@ namespace Microsoft.DotNet.UpgradeAssistant.Cli
             {
                 _logger.LogDebug("Configuration loaded from context base directory: {BaseDirectory}", AppContext.BaseDirectory);
 
-                await RunStartupAsync(token);
-                await RunCommandAsync(token);
+                using var scope = _services.GetAutofacRoot().BeginLifetimeScope(builder =>
+                {
+                    foreach (var extension in _services.GetRequiredService<IEnumerable<ExtensionInstance>>())
+                    {
+                        var services = new ServiceCollection();
+                        extension.AddServices(services);
+                        builder.Populate(services);
+                    }
+                });
+
+                await RunStartupAsync(scope.Resolve<IEnumerable<IUpgradeStartup>>(), token);
+
+                await scope.Resolve<IAppCommand>().RunAsync(token);
 
                 _errorCode.ErrorCode = ErrorCodes.Success;
             }
@@ -65,11 +79,8 @@ namespace Microsoft.DotNet.UpgradeAssistant.Cli
             }
         }
 
-        private async Task RunStartupAsync(CancellationToken token)
+        private static async Task RunStartupAsync(IEnumerable<IUpgradeStartup> startups, CancellationToken token)
         {
-            using var scope = _services.CreateScope();
-            var startups = scope.ServiceProvider.GetRequiredService<IEnumerable<IUpgradeStartup>>();
-
             foreach (var startup in startups)
             {
                 if (!await startup.StartupAsync(token))
@@ -79,13 +90,6 @@ namespace Microsoft.DotNet.UpgradeAssistant.Cli
             }
         }
 
-        private async Task RunCommandAsync(CancellationToken token)
-        {
-            using var scope = _services.CreateScope();
-            var command = scope.ServiceProvider.GetRequiredService<IAppCommand>();
-            await command.RunAsync(token);
-        }
-
         public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask;
     }
 }

src/components/Microsoft.DotNet.UpgradeAssistant.Extensions/ExtensionInstance.cs

@@ -2,12 +2,12 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
-using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using System.Runtime.Loader;
 using Microsoft.Extensions.Configuration;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.FileProviders;
 
 namespace Microsoft.DotNet.UpgradeAssistant.Extensions
@@ -40,18 +40,23 @@ namespace Microsoft.DotNet.UpgradeAssistant.Extensions
 
         public string Name { get; }
 
-        public IEnumerable<IExtensionServiceProvider> GetServiceProviders()
+        public void AddServices(IServiceCollection services)
         {
             if (_alc is null)
             {
-                return Enumerable.Empty<IExtensionServiceProvider>();
+                return;
             }
 
-            return _alc.Value.Assemblies.SelectMany(assembly => assembly
-                .GetTypes()
+            var serviceProviders = _alc.Value.Assemblies
+                .SelectMany(assembly => assembly.GetTypes()
                 .Where(t => t.IsPublic && !t.IsAbstract && typeof(IExtensionServiceProvider).IsAssignableFrom(t))
                 .Select(t => Activator.CreateInstance(t))
                 .Cast<IExtensionServiceProvider>());
+
+            foreach (var sp in serviceProviders)
+            {
+                sp.AddServices(new ExtensionServiceCollection(services, this));
+            }
         }
 
         public string Location { get; }
@@ -72,56 +77,6 @@ namespace Microsoft.DotNet.UpgradeAssistant.Extensions
             }
         }
 
-        [System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Calling method will handle disposal.")]
-        [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Creating extensions should not throw any extensions.")]
-        public static ExtensionInstance? Create(string e)
-        {
-            try
-            {
-                if (Directory.Exists(e))
-                {
-                    return new ExtensionInstance(new PhysicalFileProvider(e), e);
-                }
-                else if (File.Exists(e))
-                {
-                    if (ManifestFileName.Equals(Path.GetFileName(e), StringComparison.OrdinalIgnoreCase))
-                    {
-                        var dir = Path.GetDirectoryName(e) ?? string.Empty;
-                        return new ExtensionInstance(new PhysicalFileProvider(dir), dir);
-                    }
-                    else if (Path.GetExtension(e).Equals(".zip", StringComparison.OrdinalIgnoreCase))
-                    {
-                        var provider = new ZipFileProvider(e);
-
-                        try
-                        {
-                            return new ExtensionInstance(provider, e);
-                        }
-
-                        // If the manifest file couldn't be found, let's try looking at one layer deep with the name
-                        // of the file as the first folder. This is what happens when you create a zip file from a folder
-                        // with Windows or 7-zip
-                        catch (UpgradeException ex) when (ex.InnerException is FileNotFoundException)
-                        {
-                            var subpath = Path.GetFileNameWithoutExtension(e);
-                            var subprovider = new SubFileProvider(provider, subpath);
-                            return new ExtensionInstance(subprovider, e);
-                        }
-                    }
-                }
-                else
-                {
-                    Console.Error.WriteLine($"ERROR: Extension {e} not found; ignoring extension {e}");
-                }
-            }
-            catch (Exception ex)
-            {
-                Console.Error.WriteLine($"ERROR: Could not load extension from {e}: {ex.Message}");
-            }
-
-            return null;
-        }
-
         private static string GetName(IConfiguration configuration, string location)
         {
             if (configuration[ExtensionNamePropertyName] is string name)

src/components/Microsoft.DotNet.UpgradeAssistant.Extensions/ExtensionLoaders/DirectoryExtensionLoader.cs

@@ -0,0 +1,22 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.IO;
+using Microsoft.Extensions.FileProviders;
+
+namespace Microsoft.DotNet.UpgradeAssistant.Extensions
+{
+    internal class DirectoryExtensionLoader : IExtensionLoader
+    {
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "The file provider will be disposed when the extension instance is disposed.")]
+        public ExtensionInstance? LoadExtension(string path)
+        {
+            if (Directory.Exists(path))
+            {
+                return new ExtensionInstance(new PhysicalFileProvider(path), path);
+            }
+
+            return null;
+        }
+    }
+}

src/components/Microsoft.DotNet.UpgradeAssistant.Extensions/ExtensionLoaders/IExtensionLoader.cs

@@ -0,0 +1,10 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.DotNet.UpgradeAssistant.Extensions
+{
+    internal interface IExtensionLoader
+    {
+        ExtensionInstance? LoadExtension(string path);
+    }
+}

src/components/Microsoft.DotNet.UpgradeAssistant.Extensions/ExtensionLoaders/ManifestDirectoryExtensionLoader.cs

@@ -0,0 +1,30 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.IO;
+using Microsoft.Extensions.FileProviders;
+
+namespace Microsoft.DotNet.UpgradeAssistant.Extensions
+{
+    internal class ManifestDirectoryExtensionLoader : IExtensionLoader
+    {
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "The file provider will be disposed when the extension instance is disposed.")]
+        public ExtensionInstance? LoadExtension(string path)
+        {
+            if (!File.Exists(path))
+            {
+                return null;
+            }
+
+            var filename = Path.GetFileName(path);
+            if (ExtensionInstance.ManifestFileName.Equals(filename, StringComparison.OrdinalIgnoreCase))
+            {
+                var dir = Path.GetDirectoryName(path) ?? string.Empty;
+                return new ExtensionInstance(new PhysicalFileProvider(dir), dir);
+            }
+
+            return null;
+        }
+    }
+}

src/components/Microsoft.DotNet.UpgradeAssistant.Extensions/ExtensionLoaders/ZipExtensionLoader.cs

@@ -0,0 +1,42 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.IO;
+
+namespace Microsoft.DotNet.UpgradeAssistant.Extensions
+{
+    internal class ZipExtensionLoader : IExtensionLoader
+    {
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "The file provider will be disposed when the extension instance is disposed.")]
+        public ExtensionInstance? LoadExtension(string path)
+        {
+            if (!File.Exists(path))
+            {
+                return null;
+            }
+
+            if (Path.GetExtension(path).Equals(".zip", StringComparison.OrdinalIgnoreCase))
+            {
+                var provider = new ZipFileProvider(path);
+
+                try
+                {
+                    return new ExtensionInstance(provider, path);
+                }
+
+                // If the manifest file couldn't be found, let's try looking at one layer deep with the name
+                // of the file as the first folder. This is what happens when you create a zip file from a folder
+                // with Windows or 7-zip
+                catch (UpgradeException ex) when (ex.InnerException is FileNotFoundException)
+                {
+                    var subpath = Path.GetFileNameWithoutExtension(path);
+                    var subprovider = new SubFileProvider(provider, subpath);
+                    return new ExtensionInstance(subprovider, path);
+                }
+            }
+
+            return null;
+        }
+    }
+}

src/components/Microsoft.DotNet.UpgradeAssistant.Extensions/ExtensionLoggingStartup.cs

@@ -1,43 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// The .NET Foundation licenses this file to you under the MIT license.
-
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Threading;
-using System.Threading.Tasks;
-using Microsoft.Extensions.Logging;
-
-namespace Microsoft.DotNet.UpgradeAssistant.Extensions
-{
-    internal class ExtensionLoggingStartup : IUpgradeStartup
-    {
-        private readonly ILogger<ExtensionLoggingStartup> _logger;
-        private readonly IEnumerable<ExtensionInstance> _extensions;
-
-        public ExtensionLoggingStartup(ILogger<ExtensionLoggingStartup> logger, IEnumerable<ExtensionInstance> extensions)
-        {
-            _logger = logger;
-            _extensions = extensions;
-        }
-
-        public Task<bool> StartupAsync(CancellationToken token)
-        {
-            _logger.LogInformation("Loaded {0} extensions", _extensions.Count());
-
-            foreach (var extension in _extensions)
-            {
-                if (extension.Version is Version version)
-                {
-                    _logger.LogDebug("Loaded extension: {Name} v{Version} [{Location}]", extension.Name, version, extension.Location);
-                }
-                else
-                {
-                    _logger.LogDebug("Loaded extension: {Name} [{Location}]", extension.Name, extension.Location);
-                }
-            }
-
-            return Task.FromResult(true);
-        }
-    }
-}

src/components/Microsoft.DotNet.UpgradeAssistant.Extensions/ExtensionManager.cs

@@ -0,0 +1,87 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Collections.Generic;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
+
+namespace Microsoft.DotNet.UpgradeAssistant.Extensions
+{
+    internal sealed class ExtensionManager : IDisposable, IEnumerable<ExtensionInstance>
+    {
+        private readonly Lazy<IEnumerable<ExtensionInstance>> _extensions;
+
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Loading an extension should not propogate any exceptions.")]
+        public ExtensionManager(
+            IEnumerable<IExtensionLoader> loaders,
+            IOptions<ExtensionOptions> options,
+            ILogger<ExtensionManager> logger)
+        {
+            _extensions = new Lazy<IEnumerable<ExtensionInstance>>(() =>
+            {
+                var list = new List<ExtensionInstance>();
+
+                foreach (var path in options.Value.ExtensionPaths)
+                {
+                    if (LoadExtension(path) is ExtensionInstance extension)
+                    {
+                        list.Add(extension);
+                    }
+                    else
+                    {
+                        logger.LogWarning("Could not load extension from {Path}", path);
+                    }
+                }
+
+                logger.LogInformation("Loaded {Count} extensions", list.Count);
+
+                return list;
+            });
+
+            ExtensionInstance? LoadExtension(string path)
+            {
+                foreach (var loader in loaders)
+                {
+                    try
+                    {
+                        if (loader.LoadExtension(path) is ExtensionInstance instance)
+                        {
+                            if (instance.Version is Version version)
+                            {
+                                logger.LogDebug("Loaded extension: {Name} v{Version} [{Location}]", instance.Name, version, instance.Location);
+                            }
+                            else
+                            {
+                                logger.LogDebug("Loaded extension: {Name} [{Location}]", instance.Name, instance.Location);
+                            }
+
+                            return instance;
+                        }
+                    }
+                    catch (Exception e)
+                    {
+                        logger.LogError(e, "There was an error loading an extension from {Path}", path);
+                    }
+                }
+
+                return null;
+            }
+        }
+
+        public void Dispose()
+        {
+            if (_extensions.IsValueCreated)
+            {
+                foreach (var extension in _extensions.Value)
+                {
+                    extension.Dispose();
+                }
+            }
+        }
+
+        public IEnumerator<ExtensionInstance> GetEnumerator() => _extensions.Value.GetEnumerator();
+
+        System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => GetEnumerator();
+    }
+}

src/components/Microsoft.DotNet.UpgradeAssistant.Extensions/ExtensionMappedFileConfigureOptions{TOption,TTo}.cs

@@ -8,7 +8,6 @@ using System.Text.Json;
 using Microsoft.Extensions.FileProviders;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
-using Microsoft.Extensions.Primitives;
 
 namespace Microsoft.DotNet.UpgradeAssistant.Extensions
 {

src/components/Microsoft.DotNet.UpgradeAssistant.Extensions/ExtensionOptions.cs

@@ -0,0 +1,12 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections.Generic;
+
+namespace Microsoft.DotNet.UpgradeAssistant.Extensions
+{
+    public class ExtensionOptions
+    {
+        public ICollection<string> ExtensionPaths { get; } = new List<string>();
+    }
+}

src/components/Microsoft.DotNet.UpgradeAssistant.Extensions/ExtensionProviderExtensions.cs

@@ -9,6 +9,7 @@ using System.Text.Json;
 using System.Text.Json.Serialization;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Options;
 
 namespace Microsoft.DotNet.UpgradeAssistant.Extensions
 {
@@ -35,22 +36,17 @@ namespace Microsoft.DotNet.UpgradeAssistant.Extensions
                 throw new ArgumentNullException(nameof(configuration));
             }
 
-            services.AddTransient<IUpgradeStartup, ExtensionLoggingStartup>();
-            services.AddSerializer();
-
-            foreach (var extension in GetExtensions(configuration, additionalExtensionPaths))
-            {
-                services.AddSingleton(extension);
-
-                foreach (var sp in extension.GetServiceProviders())
+            services.AddOptions<ExtensionOptions>()
+                .AddDefaultExtensions(configuration)
+                .AddFromEnvironmentVariables(configuration)
+                .Configure(options =>
                 {
-                    sp.AddServices(new ExtensionServiceCollection(services, extension));
-                }
-            }
-        }
+                    foreach (var path in additionalExtensionPaths)
+                    {
+                        options.ExtensionPaths.Add(path);
+                    }
+                });
 
-        private static void AddSerializer(this IServiceCollection services)
-        {
             services.AddOptions<JsonSerializerOptions>()
                 .Configure(o =>
                 {
@@ -58,37 +54,45 @@ namespace Microsoft.DotNet.UpgradeAssistant.Extensions
                     o.ReadCommentHandling = JsonCommentHandling.Skip;
                     o.Converters.Add(new JsonStringEnumConverter());
                 });
+
+            services.AddScoped<ExtensionManager>();
+            services.AddTransient<IEnumerable<ExtensionInstance>>(ctx => ctx.GetRequiredService<ExtensionManager>());
+            services.AddExtensionLoaders();
         }
 
-        [System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Will be disposed by dependency injection.")]
-        private static IEnumerable<ExtensionInstance> GetExtensions(IConfiguration originalConfiguration, IEnumerable<string> additionalExtensionPaths)
+        private static void AddExtensionLoaders(this IServiceCollection services)
         {
-            foreach (var e in GetExtensionPaths())
-            {
-                if (string.IsNullOrEmpty(e))
-                {
-                    continue;
-                }
+            services.AddTransient<IExtensionLoader, DirectoryExtensionLoader>();
+            services.AddTransient<IExtensionLoader, ManifestDirectoryExtensionLoader>();
+            services.AddTransient<IExtensionLoader, ZipExtensionLoader>();
+        }
 
-                if (ExtensionInstance.Create(e) is ExtensionInstance instance)
-                {
-                    yield return instance;
-                }
-            }
-
-            IEnumerable<string> GetExtensionPaths()
+        private static OptionsBuilder<ExtensionOptions> AddDefaultExtensions(this OptionsBuilder<ExtensionOptions> builder, IConfiguration configuration)
+            => builder.Configure(options =>
             {
                 const string ExtensionDirectory = "extensions";
+                const string DefaultExtensionsSection = "DefaultExtensions";
 
-                var fromConfig = originalConfiguration.GetSection("DefaultExtensions")
+                var defaultExtensions = configuration.GetSection(DefaultExtensionsSection)
                     .Get<string[]>()
                     .Select(n => Path.GetFullPath(Path.Combine(AppContext.BaseDirectory, ExtensionDirectory, n)));
 
-                var extensionPathString = originalConfiguration[UpgradeAssistantExtensionPathsSettingName];
+                foreach (var path in defaultExtensions)
+                {
+                    options.ExtensionPaths.Add(path);
+                }
+            });
+
+        private static OptionsBuilder<ExtensionOptions> AddFromEnvironmentVariables(this OptionsBuilder<ExtensionOptions> builder, IConfiguration configuration)
+            => builder.Configure(options =>
+            {
+                var extensionPathString = configuration[UpgradeAssistantExtensionPathsSettingName];
                 var pathsFromString = extensionPathString?.Split(new char[] { ';' }, StringSplitOptions.RemoveEmptyEntries) ?? Enumerable.Empty<string>();
 
-                return fromConfig.Concat(pathsFromString).Concat(additionalExtensionPaths);
-            }
-        }
+                foreach (var path in pathsFromString)
+                {
+                    options.ExtensionPaths.Add(path);
+                }
+            });
     }
 }