(refactor): code review

This commit is contained in:
Maksym Sadovnychyy 2023-12-28 12:46:09 +01:00
parent 6d996ae3c8
commit c52e3db122
13 changed files with 151 additions and 73 deletions

View File

@ -29,7 +29,7 @@ I have also prepared ***.cmd** file to simplify service system integration:
Install.cmd
```bat
sc.exe create "Unified Scheduler Service" binpath="%~dp0UScheduler.exe
sc.exe create "Unified Scheduler Service" binpath="%~dp0UScheduler.exe"
pause
```

View File

@ -30,23 +30,18 @@ namespace UScheduler.BackgroundServices {
//stop background service if there are no PowerShell scripts to run
if (psScripts.Count == 0) {
_logger.LogInformation("No PowerShell scripts to run, stopping PSScriptBackgroundService");
_logger.LogWarning("No PowerShell scripts to run, stopping PSScriptBackgroundService");
break;
}
foreach (var psScript in psScripts) {
if (psScript.GetPathOrDefault == string.Empty)
continue;
var scriptPath = psScript.GetPathOrDefault;
if (_psScriptService.GetRunningScriptTasks().Contains(scriptPath)) {
_logger.LogInformation($"PowerShell script {scriptPath} is already running");
if (scriptPath == string.Empty)
continue;
}
_logger.LogInformation($"Running PowerShell script {scriptPath}");
_psScriptService.RunScript(scriptPath, psScript.GetIsSignedOrDefault);
_psScriptService.RunScript(scriptPath, psScript.GetIsSignedOrDefault, stoppingToken);
}
await Task.Delay(TimeSpan.FromSeconds(10), stoppingToken);
@ -56,8 +51,6 @@ namespace UScheduler.BackgroundServices {
// When the stopping token is canceled, for example, a call made from services.msc,
// we shouldn't exit with a non-zero exit code. In other words, this is expected...
_logger.LogInformation("Stopping PSScriptBackgroundService due to cancellation request");
_psScriptService.TerminateAllScripts();
}
catch (Exception ex) {
_logger.LogError(ex, "{Message}", ex.Message);
@ -73,5 +66,17 @@ namespace UScheduler.BackgroundServices {
Environment.Exit(1);
}
}
public override Task StopAsync(CancellationToken stoppingToken) {
// Perform cleanup tasks here
_logger.LogInformation("Stopping PSScriptBackgroundService");
_psScriptService.TerminateAllScripts();
_logger.LogInformation("PSScriptBackgroundService stopped");
return Task.CompletedTask;
}
}
}

View File

@ -1,4 +1,5 @@
using Microsoft.Extensions.Options;
using System.Text.Json;
using UScheduler.Services;
namespace UScheduler.BackgroundServices;
@ -30,25 +31,22 @@ public sealed class ProcessBackgroundService : BackgroundService {
//stop background service if there are no processes to run
if (processes.Count == 0) {
_logger.LogInformation("No processes to run, stopping ProcessBackgroundService");
_logger.LogWarning("No processes to run, stopping ProcessBackgroundService");
break;
}
foreach (var process in processes) {
if (process.GetPathOrDefault == string.Empty)
continue;
var processPath = process.GetPathOrDefault;
var processArgs = process.GetArgsOrDefault;
if (_processService.GetRunningProcesses().Any(x => x.Value.StartInfo.FileName == processPath)) {
_logger.LogInformation($"Process {processPath} is already running");
if (processPath == string.Empty)
continue;
}
_logger.LogInformation($"Running process {processPath} with arguments {string.Join(", ", processArgs)}");
_processService.RunProcess(processPath, processArgs, stoppingToken);
}
await Task.Delay(TimeSpan.FromSeconds(10), stoppingToken);
}
@ -57,8 +55,6 @@ public sealed class ProcessBackgroundService : BackgroundService {
// When the stopping token is canceled, for example, a call made from services.msc,
// we shouldn't exit with a non-zero exit code. In other words, this is expected...
_logger.LogInformation("Stopping ProcessBackgroundService due to cancellation request");
_processService.TerminateAllProcesses();
}
catch (Exception ex) {
_logger.LogError(ex, "{Message}", ex.Message);
@ -74,4 +70,15 @@ public sealed class ProcessBackgroundService : BackgroundService {
Environment.Exit(1);
}
}
public override Task StopAsync(CancellationToken stoppingToken) {
// Perform cleanup tasks here
_logger.LogInformation("Stopping ProcessBackgroundService");
_processService.TerminateAllProcesses();
_logger.LogInformation("All processes terminated");
return Task.CompletedTask;
}
}

View File

@ -27,20 +27,15 @@ namespace UScheduler {
public class Configuration {
public string? ServiceName { get; set; }
public string? Description { get; set; }
public string? DisplayName { get; set; }
public List<PowershellScript>? Powershell { get; set; }
public List<ProcessConfiguration>? Processes { get; set; }
public string ServiceNameOrDefault => ServiceName ?? string.Empty;
public string DescriptionOrDefault => Description ?? string.Empty;
public string DisplayNameOrDefault => DisplayName ?? string.Empty;
public List<PowershellScript> PowershellOrDefault => Powershell ?? [];
public List<PowershellScript> PowershellOrDefault => Powershell ?? new List<PowershellScript>();
public List<ProcessConfiguration> ProcessesOrDefault => Processes ?? new List<ProcessConfiguration>();
public List<ProcessConfiguration> ProcessesOrDefault => Processes ?? [];
}
}

View File

@ -1,5 +1,3 @@
"%~dp0PSScriptsService.exe" install
sc.exe create ".NET Joke Service" binpath="C:\Path\To\App.WindowsService.exe"
sc.exe create "Svc Name" binpath="C:\Path\To\App.exe --contentRoot C:\Other\Path"
sc.exe create "Unified Scheduler Service" binpath="%~dp0UScheduler.exe"
sc description "Unified Scheduler Service" "Windows service, which allows you to invoke PowerShell Scripts and Processes"
pause

View File

@ -1,13 +1,15 @@
using Microsoft.Extensions.Logging.Configuration;
using Microsoft.Extensions.Logging.EventLog;
using System.Runtime.InteropServices;
using UScheduler;
using UScheduler.BackgroundServices;
using UScheduler.Services;
// read configuration from appsettings.json
var configurationRoot = new ConfigurationBuilder()
.SetBasePath(Directory.GetCurrentDirectory())
.SetBasePath(AppDomain.CurrentDomain.BaseDirectory)
.AddJsonFile("appsettings.json", optional: true)
.AddJsonFile($"appsettings.{Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT") ?? "Production"}.json", optional: true)
.Build();
// bind Configuration section inside configuration to a new instance of Settings
@ -19,13 +21,11 @@ builder.Services.AddWindowsService(options => {
options.ServiceName = configuration.ServiceNameOrDefault;
});
LoggerProviderOptions.RegisterProviderOptions<
EventLogSettings, EventLogLoggerProvider>(builder.Services);
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) {
LoggerProviderOptions.RegisterProviderOptions<
EventLogSettings, EventLogLoggerProvider>(builder.Services);
}
// register configuration as IOptions<Configuration>
builder.Services.Configure<Configuration>(configurationRoot.GetSection("Configurations"));
builder.Services.AddSingleton<ProcessService>();

View File

@ -0,0 +1,18 @@
<?xml version="1.0" encoding="utf-8"?>
<!--
https://go.microsoft.com/fwlink/?LinkID=208121.
-->
<Project>
<PropertyGroup>
<Configuration>Release</Configuration>
<Platform>Any CPU</Platform>
<PublishDir>bin\Release\net8.0\win-x64\publish\win-x64\</PublishDir>
<PublishProtocol>FileSystem</PublishProtocol>
<_TargetId>Folder</_TargetId>
<TargetFramework>net8.0</TargetFramework>
<RuntimeIdentifier>win-x64</RuntimeIdentifier>
<SelfContained>true</SelfContained>
<PublishReadyToRun>true</PublishReadyToRun>
<PublishTrimmed>false</PublishTrimmed>
</PropertyGroup>
</Project>

View File

@ -17,9 +17,14 @@ namespace UScheduler.Services {
}
}
public Task RunScript(string scriptPath, bool signed) {
public Task RunScript(string scriptPath, bool signed, CancellationToken stoppingToken) {
_logger.LogInformation($"Preparing to run script {scriptPath}");
if (GetRunningScriptTasks().Contains(scriptPath)) {
_logger.LogInformation($"PowerShell script {scriptPath} is already running");
return Task.CompletedTask;
}
if (!File.Exists(scriptPath)) {
_logger.LogError($"Script file {scriptPath} does not exist");
return Task.CompletedTask;
@ -60,11 +65,17 @@ namespace UScheduler.Services {
ps.Invoke();
}
}
catch (OperationCanceledException) {
// When the stopping token is canceled, for example, a call made from services.msc,
// we shouldn't exit with a non-zero exit code. In other words, this is expected...
_logger.LogInformation($"Stopping script {scriptPath} due to cancellation request");
}
catch (Exception ex) {
_logger.LogError($"Error running script {scriptPath}: {ex.Message}");
}
finally {
_runningScripts.TryRemove(scriptPath, out _);
TerminateScript(scriptPath);
_logger.LogInformation($"Script {scriptPath} completed and removed from running scripts");
}

View File

@ -16,11 +16,20 @@ namespace UScheduler.Services {
Process? process = null;
try {
if (GetRunningProcesses().Any(x => x.Value.StartInfo.FileName == processPath)) {
_logger.LogInformation($"Process {processPath} is already running");
return;
}
process = new Process();
process.StartInfo.FileName = processPath;
process.StartInfo.UseShellExecute = false;
process.StartInfo.RedirectStandardOutput = true;
process.StartInfo.RedirectStandardError = true;
process.StartInfo = new ProcessStartInfo {
FileName = processPath,
WorkingDirectory = Path.GetDirectoryName(processPath),
UseShellExecute = false,
RedirectStandardOutput = true,
RedirectStandardError = true
};
foreach (var arg in args)
process.StartInfo.ArgumentList.Add(arg);
@ -41,14 +50,17 @@ namespace UScheduler.Services {
}
}
catch (OperationCanceledException) {
_logger.LogInformation($"Process {processPath} was cancelled");
// When the stopping token is canceled, for example, a call made from services.msc,
// we shouldn't exit with a non-zero exit code. In other words, this is expected...
_logger.LogWarning($"Process {processPath} was canceled");
}
catch (Exception ex) {
_logger.LogError($"Error running process {processPath}: {ex.Message}");
}
finally {
if (process != null && _runningProcesses.ContainsKey(process.Id)) {
_runningProcesses.TryRemove(process.Id, out _);
TerminateProcessById(process.Id);
_logger.LogInformation($"Process {processPath} with ID {process.Id} removed from running processes");
}
}
@ -60,16 +72,29 @@ namespace UScheduler.Services {
}
public void TerminateProcessById(int processId) {
if (_runningProcesses.TryRemove(processId, out var process)) {
_logger.LogInformation($"Terminating process with ID {processId}");
process.Kill();
_logger.LogInformation($"Process with ID {processId} terminated");
// Check if the process is in the running processes list
if (!_runningProcesses.TryGetValue(processId, out var processToTerminate)) {
_logger.LogWarning($"Failed to terminate process {processId}. Process not found.");
return;
}
else {
_logger.LogWarning($"Failed to terminate process with ID {processId}. Process not found.");
// Kill the process
try {
processToTerminate.Kill(true);
_logger.LogInformation($"Process {processId} terminated");
}
catch (Exception ex) {
_logger.LogError($"Error terminating process {processId}: {ex.Message}");
}
// Check if the process has exited
if (!processToTerminate.HasExited) {
_logger.LogWarning($"Failed to terminate process {processId}. Process still running.");
TerminateProcessById(processId);
}
}
public void TerminateAllProcesses() {
foreach (var process in _runningProcesses) {
TerminateProcessById(process.Key);

View File

@ -24,6 +24,15 @@
</ItemGroup>
<ItemGroup>
<Folder Include="Abstractions\" />
<None Update="Install.cmd">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="Uninstall.cmd">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
</ItemGroup>
<ItemGroup>
<Folder Include="Properties\PublishProfiles\" />
</ItemGroup>
</Project>

View File

@ -1,5 +1,2 @@
"%~dp0PSScriptsService.exe" uninstall
sc.exe delete ".NET Joke Service"
sc.exe delete "Unified Scheduler Service"
pause

View File

@ -1,8 +1,31 @@
{
"Logging": {
"LogLevel": {
"Default": "Information",
"Microsoft.Hosting.Lifetime": "Information"
"Default": "Warning"
},
"EventLog": {
"SourceName": "UScheduler",
"LogName": "Application",
"LogLevel": {
"Microsoft": "Information",
"Microsoft.Hosting.Lifetime": "Information"
}
}
},
"Configurations": {
"ServiceName": "UScheduler",
"Powershell": [
],
"Processes": [
{
"Path": "C:\\Users\\maksym\\Desktop\\Programs\\syncthing-windows-amd64-v1.27.1\\syncthing.exe",
"Args": ["--no-restart", "--home=C:\\Users\\maksym\\Desktop\\Data\\Syncthing"],
"RestartOnFailure": true
}
]
}
}
}

View File

@ -14,24 +14,14 @@
},
"Configurations": {
"ServiceName": "UScheduler",
"Description": "Windows service, which allows you to invoke PowerShell Scripts and Processes",
"DisplayName": "Unified Scheduler Service",
"Powershell": [
{
"Path": "",
"StartScript": "",
"Signed": true
}
],
"Processes": [
{
"Path": "C:\\Users\\maksym\\Desktop\\syncthing-windows-amd64-v1.27.1\\syncthing.exe",
"Args": [],
"RestartOnFailure": true
}
]
}
}