Merge pull request #701 from immense/tech/remove-agent-staticdependen…

…cies

Remove static dependencies and set nullability warnings to errors.

.editorconfig

@@ -96,6 +96,21 @@ dotnet_style_prefer_simplified_interpolation = true:suggestion
 # CA1822: Mark members as static
 dotnet_diagnostic.CA1822.severity = silent
 dotnet_style_namespace_match_folder = true:error
+dotnet_style_predefined_type_for_locals_parameters_members = true:silent
+dotnet_style_readonly_field = true:suggestion
+dotnet_code_quality_unused_parameters = all:suggestion
+dotnet_style_allow_statement_immediately_after_block_experimental = true:silent
+dotnet_style_allow_multiple_blank_lines_experimental = true:silent
+dotnet_style_require_accessibility_modifiers = for_non_interface_members:silent
+dotnet_style_predefined_type_for_member_access = true:silent
+dotnet_style_parentheses_in_other_operators = never_if_unnecessary:silent
+dotnet_style_parentheses_in_relational_binary_operators = always_for_clarity:silent
+dotnet_style_parentheses_in_other_binary_operators = always_for_clarity:silent
+dotnet_style_parentheses_in_arithmetic_binary_operators = always_for_clarity:silent
+dotnet_style_qualification_for_event = false:silent
+dotnet_style_qualification_for_method = false:silent
+dotnet_style_qualification_for_property = false:silent
+dotnet_style_qualification_for_field = false:silent
 [*.cs]
 csharp_indent_labels = one_less_than_current
 csharp_using_directive_placement = outside_namespace:silent
@@ -112,4 +127,43 @@ csharp_style_expression_bodied_indexers = true:silent
 csharp_style_expression_bodied_accessors = true:silent
 csharp_style_expression_bodied_lambdas = true:silent
 csharp_style_expression_bodied_local_functions = false:silent
-csharp_prefer_static_local_function = true:suggestion
+csharp_prefer_static_local_function = true:suggestion
+csharp_style_throw_expression = true:suggestion
+csharp_style_unused_value_expression_statement_preference = discard_variable:silent
+csharp_style_unused_value_assignment_preference = discard_variable:suggestion
+csharp_style_deconstructed_variable_declaration = true:suggestion
+csharp_style_inlined_variable_declaration = true:suggestion
+csharp_style_prefer_utf8_string_literals = true:suggestion
+csharp_style_prefer_tuple_swap = true:suggestion
+csharp_style_implicit_object_creation_when_type_is_apparent = true:suggestion
+csharp_style_prefer_range_operator = true:suggestion
+csharp_style_prefer_index_operator = true:suggestion
+csharp_style_prefer_local_over_anonymous_function = true:suggestion
+csharp_prefer_simple_default_expression = true:suggestion
+csharp_style_prefer_null_check_over_type_check = true:suggestion
+csharp_style_conditional_delegate_call = true:suggestion
+csharp_style_allow_blank_line_after_token_in_arrow_expression_clause_experimental = true:silent
+csharp_style_allow_blank_line_after_token_in_conditional_expression_experimental = true:silent
+csharp_style_allow_blank_line_after_colon_in_constructor_initializer_experimental = true:silent
+csharp_style_allow_blank_lines_between_consecutive_braces_experimental = true:silent
+csharp_style_allow_embedded_statements_on_same_line_experimental = true:silent
+csharp_style_prefer_readonly_struct_member = true:suggestion
+csharp_style_prefer_readonly_struct = true:suggestion
+csharp_style_prefer_pattern_matching = true:silent
+csharp_style_prefer_switch_expression = true:suggestion
+csharp_style_pattern_matching_over_is_with_cast_check = true:suggestion
+csharp_style_prefer_extended_property_pattern = true:suggestion
+csharp_style_prefer_not_pattern = true:suggestion
+csharp_style_pattern_matching_over_as_with_null_check = true:suggestion
+csharp_style_var_elsewhere = false:silent
+csharp_style_var_when_type_is_apparent = false:silent
+csharp_style_var_for_built_in_types = false:silent
+
+# CS8604: Possible null reference argument.
+dotnet_diagnostic.CS8604.severity = error
+
+# CS8600: Converting null literal or possible null value to non-nullable type.
+dotnet_diagnostic.CS8600.severity = error
+
+# CS8602: Dereference of a possibly null reference.
+dotnet_diagnostic.CS8602.severity = error

Agent.Installer.Win/Agent.Installer.Win.csproj

@@ -15,8 +15,8 @@
     <AutoGenerateBindingRedirects>true</AutoGenerateBindingRedirects>
     <Deterministic>true</Deterministic>
     <TargetFrameworkProfile />
-	<LangVersion>10</LangVersion>
-	<Nullable>enable</Nullable>
+    <LangVersion>10</LangVersion>
+    <Nullable>enable</Nullable>
   </PropertyGroup>
   <PropertyGroup>
     <RuntimeIdentifiers>win;win-x64;win10-x64;win-x64;win10-x86;</RuntimeIdentifiers>

Agent.Installer.Win/ViewModels/MainWindowViewModel.cs

@@ -392,7 +392,7 @@ private async Task ExtractEmbeddedServerData()
                 return;
             }
 
-            var embeddedData = await _embeddedDataReader.TryGetEmbeddedData(filePath);
+            var embeddedData = await _embeddedDataReader.TryGetEmbeddedData(filePath!);
 
             if (embeddedData is null || embeddedData == EmbeddedServerData.Empty)
             {

Agent/Interfaces/IScriptingShell.cs

@@ -0,0 +1,11 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Remotely.Agent.Interfaces;
+public interface IScriptingShell
+{
+    bool IsDisposed { get; }
+}

Agent/Program.cs

@@ -23,9 +23,6 @@ namespace Remotely.Agent;
 
 public class Program
 {
-    [Obsolete("Remove this when all services are in DI behind interfaces.")]
-    public static IServiceProvider? Services { get; set; }
-
     public static async Task Main(string[] args)
     {
         try
@@ -39,8 +36,6 @@ public static async Task Main(string[] args)
 
             await host.StartAsync();
 
-            Services = host.Services;
-
             await Init(host.Services);
 
             await host.WaitForShutdownAsync();
@@ -105,6 +100,7 @@ private static void RegisterServices(IServiceCollection services)
         services.AddScoped<IProcessInvoker, ProcessInvoker>();
         services.AddScoped<IUpdateDownloader, UpdateDownloader>();
         services.AddSingleton<IFileLogsManager, FileLogsManager>();
+        services.AddSingleton<IScriptingShellFactory, ScriptingShellFactory>();
 
         if (OperatingSystem.IsWindows())
         {

Agent/Services/AgentHubConnection.cs

@@ -42,6 +42,7 @@ public class AgentHubConnection : IAgentHubConnection, IDisposable
     private readonly ILogger<AgentHubConnection> _logger;
     private readonly IFileLogsManager _fileLogsManager;
     private readonly IHostApplicationLifetime _appLifetime;
+    private readonly IScriptingShellFactory _scriptingShellFactory;
     private readonly IScriptExecutor _scriptExecutor;
     private readonly IUninstaller _uninstaller;
     private readonly IUpdater _updater;
@@ -63,6 +64,7 @@ public AgentHubConnection(
         IWakeOnLanService wakeOnLanService,
         IFileLogsManager fileLogsManager,
         IHostApplicationLifetime appLifetime,
+        IScriptingShellFactory scriptingShellFactory,
         ILogger<AgentHubConnection> logger)
     {
         _configService = configService;
@@ -77,6 +79,7 @@ public AgentHubConnection(
         _logger = logger;
         _fileLogsManager = fileLogsManager;
         _appLifetime = appLifetime;
+        _scriptingShellFactory = scriptingShellFactory;
     }
 
     public bool IsConnected => _hubConnection?.State == HubConnectionState.Connected;
@@ -395,7 +398,7 @@ private void RegisterMessageHandlers()
         {
             try
             {
-                var session = PsCoreShell.GetCurrent(senderConnectionId);
+                var session = _scriptingShellFactory.GetOrCreatePsCoreShell(senderConnectionId);
                 var completion = session.GetCompletions(inputText, currentIndex, forward);
                 var completionModel = completion.ToPwshCompletion();
                 await _hubConnection.InvokeAsync("ReturnPowerShellCompletions", completionModel, intent, senderConnectionId).ConfigureAwait(false);

Agent/Services/ExternalScriptingShell.cs

@@ -1,9 +1,7 @@
-using Immense.RemoteControl.Shared.Extensions;
-using Microsoft.Extensions.DependencyInjection;
-using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging;
+using Remotely.Agent.Interfaces;
 using Remotely.Shared.Dtos;
 using Remotely.Shared.Enums;
-using Remotely.Shared.Models;
 using System;
 using System.Collections.Concurrent;
 using System.Diagnostics;
@@ -14,7 +12,7 @@
 
 namespace Remotely.Agent.Services;
 
-public interface IExternalScriptingShell
+public interface IExternalScriptingShell : IDisposable, IScriptingShell
 {
     Process? ShellProcess { get; }
     Task Init(ScriptingShell shell, string shellProcessName, string lineEnding, string connectionId);
@@ -28,6 +26,7 @@ public class ExternalScriptingShell : IExternalScriptingShell
     private readonly ILogger<ExternalScriptingShell> _logger;
     private readonly ManualResetEvent _outputDone = new(false);
     private readonly SemaphoreSlim _writeLock = new(1, 1);
+    private bool _disposedValue;
     private string _errorOut = string.Empty;
     private string _lastInputID = string.Empty;
     private string _lineEnding = Environment.NewLine;
@@ -39,7 +38,6 @@ public class ExternalScriptingShell : IExternalScriptingShell
     private string? _senderConnectionId;
     private ScriptingShell _shell;
     private string _standardOut = string.Empty;
-
     public ExternalScriptingShell(
         IConfigService configService,
         ILogger<ExternalScriptingShell> logger)
@@ -48,38 +46,14 @@ public ExternalScriptingShell(
         _logger = logger;
     }
 
-    public Process? ShellProcess { get; private set; }
+    public bool IsDisposed => _disposedValue;
 
+    public Process? ShellProcess { get; private set; }
 
-    // TODO: Turn into cache and factory.
-    public static async Task<IExternalScriptingShell> GetCurrent(ScriptingShell shell, string senderConnectionId)
+    public void Dispose()
     {
-        if (_sessions.TryGetValue($"{shell}-{senderConnectionId}", out var session) &&
-            session.ShellProcess?.HasExited != true)
-        {
-            return session;
-        }
-        else
-        {
-            session = Program.Services.GetRequiredService<IExternalScriptingShell>();
-
-            switch (shell)
-            {
-                case ScriptingShell.WinPS:
-                    await session.Init(shell, "powershell.exe", "\r\n", senderConnectionId);
-                    break;
-                case ScriptingShell.Bash:
-                    await session.Init(shell, "bash", "\n", senderConnectionId);
-                    break;
-                case ScriptingShell.CMD:
-                    await session.Init(shell, "cmd.exe", "\r\n", senderConnectionId);
-                    break;
-                default:
-                    throw new ArgumentException($"Unknown external scripting shell type: {shell}");
-            }
-            _sessions.AddOrUpdate($"{shell}-{senderConnectionId}", session, (id, b) => session);
-            return session;
-        }
+        Dispose(disposing: true);
+        GC.SuppressFinalize(this);
     }
 
     public async Task Init(ScriptingShell shell, string shellProcessName, string lineEnding, string connectionId)
@@ -187,6 +161,19 @@ public async Task<ScriptResultDto> WriteInput(string input, TimeSpan timeout)
         return GeneratePartialResult(input, sw.Elapsed);
     }
 
+    protected virtual void Dispose(bool disposing)
+    {
+        if (!_disposedValue)
+        {
+            if (disposing)
+            {
+                ShellProcess?.Dispose();
+            }
+
+            _disposedValue = true;
+        }
+    }
+
     private ScriptResultDto GenerateCompletedResult(string input, TimeSpan runtime)
     {
         return new ScriptResultDto()

Agent/Services/PsCoreShell.cs

@@ -1,5 +1,6 @@
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using Remotely.Agent.Interfaces;
 using Remotely.Shared.Dtos;
 using Remotely.Shared.Models;
 using System;
@@ -11,7 +12,7 @@
 
 namespace Remotely.Agent.Services;
 
-public interface IPsCoreShell
+public interface IPsCoreShell : IDisposable, IScriptingShell
 {
     string? SenderConnectionId { get; set; }
 
@@ -21,11 +22,11 @@ public interface IPsCoreShell
 
 public class PsCoreShell : IPsCoreShell
 {
-    private static readonly ConcurrentDictionary<string, IPsCoreShell> _sessions = new();
     private readonly IConfigService _configService;
     private readonly ConnectionInfo _connectionInfo;
     private readonly ILogger<PsCoreShell> _logger;
     private readonly PowerShell _powershell;
+    private bool _disposedValue;
     private CommandCompletion? _lastCompletion;
     private string? _lastInputText;
 
@@ -49,22 +50,13 @@ public PsCoreShell(
         _powershell.Invoke();
     }
 
+    public bool IsDisposed => _disposedValue;
     public string? SenderConnectionId { get; set; }
 
-    // TODO: Turn into cache and factory.
-    public static IPsCoreShell GetCurrent(string senderConnectionId)
+    public void Dispose()
     {
-        if (_sessions.TryGetValue(senderConnectionId, out var session))
-        {
-            return session;
-        }
-        else
-        {
-            session = Program.Services.GetRequiredService<IPsCoreShell>();
-            session.SenderConnectionId = senderConnectionId;
-            _sessions.AddOrUpdate(senderConnectionId, session, (id, b) => session);
-            return session;
-        }
+        Dispose(disposing: true);
+        GC.SuppressFinalize(this);
     }
 
     public CommandCompletion GetCompletions(string inputText, int currentIndex, bool? forward)
@@ -150,4 +142,16 @@ public async Task<ScriptResultDto> WriteInput(string input)
             };
         }
     }
+
+    protected virtual void Dispose(bool disposing)
+    {
+        if (!_disposedValue)
+        {
+            if (disposing)
+            {
+                _powershell.Dispose();
+            }
+            _disposedValue = true;
+        }
+    }
 }

Agent/Services/ScriptExecutor.cs

@@ -25,11 +25,16 @@ public interface IScriptExecutor
 public class ScriptExecutor : IScriptExecutor
 {
     private readonly IConfigService _configService;
+    private readonly IScriptingShellFactory _scriptingShellFactory;
     private readonly ILogger<ScriptExecutor> _logger;
 
-    public ScriptExecutor(IConfigService configService, ILogger<ScriptExecutor> logger)
+    public ScriptExecutor(
+        IConfigService configService, 
+        IScriptingShellFactory scriptingShellFactory,
+        ILogger<ScriptExecutor> logger)
     {
         _configService = configService;
+        _scriptingShellFactory = scriptingShellFactory;
         _logger = logger;
     }
 
@@ -161,15 +166,15 @@ private async Task<ScriptResultDto> ExecuteScriptContent(
         switch (shell)
         {
             case ScriptingShell.PSCore:
-                return await PsCoreShell
-                    .GetCurrent(terminalSessionId)
+                return await _scriptingShellFactory
+                    .GetOrCreatePsCoreShell(terminalSessionId)
                     .WriteInput(command);
 
             case ScriptingShell.WinPS:
                 if (EnvironmentHelper.IsWindows)
                 {
-                    var instance = await ExternalScriptingShell
-                      .GetCurrent(ScriptingShell.WinPS, terminalSessionId);
+                    var instance = await _scriptingShellFactory
+                      .GetOrCreateExternalShell(ScriptingShell.WinPS, terminalSessionId);
 
                     return await instance.WriteInput(command, timeout);
                 }
@@ -178,17 +183,17 @@ private async Task<ScriptResultDto> ExecuteScriptContent(
             case ScriptingShell.CMD:
                 if (EnvironmentHelper.IsWindows)
                 {
-                    var instance = await ExternalScriptingShell
-                         .GetCurrent(ScriptingShell.CMD, terminalSessionId);
+                    var instance = await _scriptingShellFactory
+                         .GetOrCreateExternalShell(ScriptingShell.CMD, terminalSessionId);
 
                     return await instance.WriteInput(command, timeout);
                 }
                 break;
 
             case ScriptingShell.Bash:
                 {
-                    var instance = await ExternalScriptingShell
-                        .GetCurrent(ScriptingShell.Bash, terminalSessionId);
+                    var instance = await _scriptingShellFactory
+                        .GetOrCreateExternalShell(ScriptingShell.Bash, terminalSessionId);
 
                     return await instance.WriteInput(command, timeout);
                 }