11 KiB
Junior Engineer Task List - Code Improvements
This document contains actionable tasks derived from the code analysis in plan.md. Complete tasks in priority order. After each fix, run:
dotnet build Inspectron.Epson.slnx
dotnet test EpsonTest/EpsonTest.csproj
CRITICAL PRIORITY (Fix First)
Task 1: Move API Key to Environment Variable
File: EpsonPrintService/config.json:4
Problem: API key stored in plaintext - security risk.
Steps:
- Remove
ApiKeyfromconfig.json - In
EpsonPrintService/Program.cs, read from environment:
var apiKey = Environment.GetEnvironmentVariable("EPSON_API_KEY")
?? throw new InvalidOperationException("EPSON_API_KEY environment variable not set");
- Update
EpsonPrintServiceConfigurationclass to accept apiKey via constructor or property - Add to systemd service file
epson.service:
[Service]
Environment="EPSON_API_KEY=your-key-here"
- Document the required environment variable in README
Task 2: Fix HttpClient Anti-Pattern
Files:
EpsonPrintService/HeartbeatBackgroundTask.cs:24Inspectron.Epson/PrintServer/PrintServices/EpsonPrintService.cs:68
Problem: Creating new HttpClient per request causes socket exhaustion.
Steps:
-
In
HeartbeatBackgroundTask.cs:- Change constructor to accept
HttpClientas parameter - Remove
_httpClient = new HttpClient();line - Keep using
_httpClientfield
- Change constructor to accept
-
In
Program.cswhere HeartbeatBackgroundTask is created:
// Create ONE HttpClient instance for the app lifetime
var httpClient = new HttpClient();
var heartbeatTask = new HeartbeatBackgroundTask(httpClient, ...);
- In
EpsonPrintService.cs:68:- Add
HttpClient _httpClientfield to class - Inject via constructor
- Replace
using var httpClient = new HttpClient();with_httpClient
- Add
Task 3: Add Graceful Shutdown
File: EpsonPrintService/Program.cs:51-82
Problem: PrintLoop and PrintServer never call StopAsync() - jobs lost on Ctrl+C.
Steps:
- Store references to started services:
var printLoop = kernel.Get<PrintLoop>();
var printServer = kernel.Get<PrintServer>();
- In the Console.CancelKeyPress handler (or before Environment.Exit):
Console.CancelKeyPress += async (sender, e) =>
{
e.Cancel = true; // Prevent immediate exit
Console.WriteLine("Shutting down gracefully...");
await printLoop.StopAsync();
await printServer.StopAsync();
cts.Cancel();
};
- Ensure
PrintLoopandPrintServerhaveStopAsync()methods that properly clean up
Task 4: Add Null Check for PrinterId
File: Inspectron.Epson/PrintServer/PrintServices/EpsonPrintService.cs:35-44
Problem: GetPrinterIdAsync() can return null but .Value accessed without check.
Steps:
- Find this code block:
var printerId = await epsonPrinter.GetPrinterIdAsync();
var printerAdapter = _wrapperPrinterFactory.CreatePrinterFromId(printerId.Value, ...);
- Add null check:
var printerId = await epsonPrinter.GetPrinterIdAsync();
if (printerId == null || !printerId.HasValue)
{
_logger.LogError("Failed to get printer ID from {PrinterIp}", printerIp);
return false;
}
var printerAdapter = _wrapperPrinterFactory.CreatePrinterFromId(printerId.Value, ...);
Task 5: Fix Race Condition in PrintServer
File: Inspectron.Epson/Queue/PrintServer.cs:19-23
Problem: TOCTOU bug - two threads could create duplicate queues.
Steps:
- Change dictionary type from
Dictionary<string, PrinterQueue>to:
private readonly ConcurrentDictionary<string, PrinterQueue> _printerQueues = new();
- Replace this pattern:
if (_printerQueues.ContainsKey(printerIp))
return;
var queue = new PrinterQueue(...);
_printerQueues[printerIp] = queue;
With atomic operation:
var queue = _printerQueues.GetOrAdd(printerIp, ip =>
{
var newQueue = new PrinterQueue(ip, _printService, _logger);
newQueue.StartAsync(); // Start the queue
Console.WriteLine($"Printer {ip} registered");
return newQueue;
});
- Add
using System.Collections.Concurrent;at top of file
HIGH PRIORITY
Task 6: Add CancellationToken Support
File: Inspectron.Epson/EpsonPrinter.cs - Lines 52, 96, 129, 205, 335, 408, etc.
Problem: Async methods can't be cancelled by callers.
Steps:
- Find all public async methods in EpsonPrinter.cs
- Add optional parameter to each:
// Before:
public async Task<bool> ConnectAsync(string host, int port = 9100)
// After:
public async Task<bool> ConnectAsync(string host, int port = 9100, CancellationToken cancellationToken = default)
- Pass cancellationToken to internal async calls:
await _stream.WriteAsync(data, cancellationToken);
await _stream.ReadAsync(buffer, cancellationToken);
- Update interface
IEpsonPrinterif one exists
Task 7: Add Exception Handling to Fire-and-Forget Tasks
Files:
Inspectron.Epson/PrintServer/PrintLoop.cs:29EpsonPrintService/Program.cs:71-72
Problem: Unobserved exceptions crash the process.
Steps:
- In
PrintLoop.cs, replace:
_ = Task.Run(Loop);
With:
_ = Task.Run(async () =>
{
try
{
await Loop();
}
catch (Exception ex)
{
_logger.LogError(ex, "PrintLoop crashed unexpectedly");
}
});
- In
Program.cs, replace:
_ = discoveryTask.StartAsync(cts.Token);
_ = heartbeatTask.StartAsync(cts.Token);
With:
var discoveryRunning = Task.Run(async () =>
{
try { await discoveryTask.StartAsync(cts.Token); }
catch (Exception ex) { Console.WriteLine($"Discovery error: {ex.Message}"); }
});
var heartbeatRunning = Task.Run(async () =>
{
try { await heartbeatTask.StartAsync(cts.Token); }
catch (Exception ex) { Console.WriteLine($"Heartbeat error: {ex.Message}"); }
});
Task 8: Implement IDisposable for Resource Cleanup
Files:
EpsonPrintService/HeartbeatBackgroundTask.csInspectron.Epson/Queue/PrinterQueue.cs:13-14Inspectron.Epson/EpsonPrinter.cs:757-761
Steps:
- HeartbeatBackgroundTask.cs - Add IDisposable:
public class HeartbeatBackgroundTask : IDisposable
{
private bool _disposed;
public void Dispose()
{
if (_disposed) return;
_httpClient?.Dispose();
_disposed = true;
}
}
- PrinterQueue.cs - Add IDisposable:
public class PrinterQueue : IDisposable
{
private bool _disposed;
public void Dispose()
{
if (_disposed) return;
_semaphore?.Dispose();
_cts?.Dispose();
_disposed = true;
}
}
- EpsonPrinter.cs - Fix DisposeAsync:
public async ValueTask DisposeAsync()
{
Disconnect();
GC.SuppressFinalize(this);
await Task.CompletedTask;
}
Task 9: Replace .Wait() with await
File: Inspectron.Epson/Queue/PrintServer.cs:32
Problem: .Wait() can cause deadlocks.
Steps:
- Find method containing:
queue.StopAsync().Wait();
- Make the method async:
// Before:
public void StopQueue(string printerIp)
// After:
public async Task StopQueueAsync(string printerIp)
- Replace
.Wait()withawait:
await queue.StopAsync();
- Update all callers to use
await StopQueueAsync()
Task 10: Add Configuration Validation at Startup
File: EpsonPrintService/Program.cs:20-21
Problem: No validation after deserializing config.
Steps:
- After config deserialization, add validation:
var config = JsonSerializer.Deserialize<EpsonPrintServiceConfiguration>(configJson);
if (config == null)
throw new InvalidOperationException("Failed to parse config.json");
if (string.IsNullOrEmpty(config.GroupId))
throw new InvalidOperationException("GroupId is required in config.json");
if (string.IsNullOrEmpty(config.RestaurantId))
throw new InvalidOperationException("RestaurantId is required in config.json");
if (config.PrinterConfigurations == null || config.PrinterConfigurations.Count == 0)
Console.WriteLine("Warning: No printer configurations found");
MEDIUM PRIORITY
Task 11: Replace Console.WriteLine with ILogger
File: Inspectron.Epson/Queue/PrintServer.cs:25, 34, 64
Replace all Console.WriteLine() calls with proper logging:
// Before:
Console.WriteLine($"Printer {printerIp} registered");
// After:
_logger.LogInformation("Printer {PrinterIp} registered", printerIp);
Task 12: Fix Logger Factory Singleton
File: EpsonPrintService/Program.cs:33-40
Create factory once:
// Create once at startup
using var loggerFactory = LoggerFactory.Create(builder =>
{
builder.AddConsole();
builder.SetMinimumLevel(LogLevel.Debug);
});
// Bind as singleton for all injections
kernel.Bind<ILoggerFactory>().ToConstant(loggerFactory);
kernel.Bind(typeof(ILogger<>)).ToMethod(ctx =>
loggerFactory.CreateLogger(ctx.Request.Service.GetGenericArguments()[0]));
Task 13: Extract Magic Numbers to Constants
File: Inspectron.Epson/Queue/PrinterQueue.cs:87-88
// Add at top of class:
private const int MaxRetryCount = 3;
// Replace:
if (job.RetryCount >= 3)
// With:
if (job.RetryCount >= MaxRetryCount)
Task 14: Fix Static Thread Safety
File: Inspectron.Epson/EpsonPrinter.cs:38-43
private static readonly object _initLock = new();
private static bool _initialized;
private static void RegisterCodepages()
{
if (_initialized) return;
lock (_initLock)
{
if (_initialized) return; // Double-check pattern
Encoding.RegisterProvider(CodePagesEncodingProvider.Instance);
_initialized = true;
}
}
LOW PRIORITY
Task 15: Remove Commented-Out Code
EpsonPrintService/Program.cs:27-28- Delete old factory bindingsInspectron.Epson/PrintServer/Printers/TM-T30III.cs:25- Delete dead code
Task 16: Resolve TODO Comments
File: Inspectron.Epson/Queue/PrintJob.cs:5
- Either change
IPproperty to proper type or remove the TODO comment
Task 17: Remove Unused Serilog Packages
File: EpsonPrintService/EpsonPrintService.csproj:14-16
dotnet remove EpsonPrintService/EpsonPrintService.csproj package Serilog
dotnet remove EpsonPrintService/EpsonPrintService.csproj package Serilog.Extensions.Logging
dotnet remove EpsonPrintService/EpsonPrintService.csproj package Serilog.Sinks.Console
Task 18: Fix Inconsistent Logging Style
Change string interpolation to structured logging:
// Before:
_logger.LogWarning($"Printer {printerIp} not found");
// After:
_logger.LogWarning("Printer {PrinterIp} not found", printerIp);
Verification Checklist
After completing all tasks:
dotnet build Inspectron.Epson.slnx- No errorsdotnet test EpsonTest/EpsonTest.csproj- All tests pass- Test printer connectivity manually
- Verify systemd service starts/stops cleanly
- Verify graceful shutdown with Ctrl+C
- Check no Console.WriteLine in library code (only logging)
Questions?
If you're unsure about any task:
- Read the original issue in
plan.mdfor more context - Look at existing patterns in the codebase
- Ask a senior engineer before making architectural changes