I'm new to C#, and want to conform as much as possible to good design patterns. This code is the beginning of a Microservice, running as Asp.Net. It is based on a Microsoft tutorial doing similar work.
It has three functional components currently:
- Converts a CSV file to JSON, for return via REST
- A REST controller to test the conversion of the CSV to JSON return
- A background service which monitors a directory looking for changed files. The file attributes (Path, Date, Checksum) are stored in a MongoDB database via a Repository.
Opinions/Recommendations regarding the design patterns welcomed.
The full code can be found at: https://github.com/BioComSoftware/unite-radimaging-source-n2m2.git
BONUS: Notice in FileSearchHostedService.cs, I instantiate the Repository objects explicitly. I think it would be more appropriate to have them as Dependency Injection - but I couldn't get it to work. Notice the commented-out lines that would have done this. My understanding is; I would need to do this with IScopedProcessingService - but I have no idea how to do this with this specific code.
[Startup.cs]
using unite.radimaging.source.n2m2.Data;
using unite.radimaging.source.n2m2.Repositories;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
//using Microsoft.OpenApi.Models;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
namespace unite.radimaging.source.n2m2 {
public class Startup {
public Startup(IConfiguration configuration) {
Configuration = configuration;
}
public IConfiguration Configuration { get; }
// This method gets called by the runtime.
// Use this method to add services to the container.
public void ConfigureServices(IServiceCollection services) {
services.AddControllers();
services.AddScoped<IFoundFileContext, FoundFileContext>();
services.AddScoped<IFoundFileRepository, FoundFileRepository>();
}
// This method gets called by the runtime.
// Use this method to configure the HTTP request pipeline.
public void Configure(IApplicationBuilder app, IWebHostEnvironment env) {
if (env.IsDevelopment()) {
app.UseDeveloperExceptionPage();
}
app.UseExceptionHandler("/error");
app.UseRouting();
app.UseAuthorization();
app.UseEndpoints(endpoints => {
endpoints.MapControllers();
});
}
}
}
[FoundFileRepository.cs]
using unite.radimaging.source.n2m2.Data;
using unite.radimaging.source.n2m2.Entities;
using MongoDB.Driver;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
namespace unite.radimaging.source.n2m2.Repositories {
public class FoundFileRepository : IFoundFileRepository {
private readonly IFoundFileContext _context;
public FoundFileRepository(IFoundFileContext context) {
_context = context ?? throw new ArgumentNullException(nameof(context));
}
public async Task<IEnumerable<FoundFile>> GetFiles() {
return await _context
.FoundFiles
.Find(p => true)
.ToListAsync();
}
public async Task<FoundFile> GetFile(string id) {
return await _context
.FoundFiles
.Find(p => p.Id == id)
.FirstOrDefaultAsync();
}
public async Task<FoundFile> GetFileByPath(string path) {
return await _context
.FoundFiles
.Find(p => p.Path == path)
.FirstOrDefaultAsync();
}
public async Task<FoundFile> GetFileByChecksum(string checksum) {
return await _context
.FoundFiles
.Find(p => p.Checksum == checksum)
.FirstOrDefaultAsync();
}
//public async Task<IEnumerable<FoundFile>> GetFileByMtime(string mtime) {
// FilterDefinition<FoundFile> filter = Builders<FoundFile>.Filter.ElemMatch<DateTime>(p => p.Mtime, mtime);
// return await _context
// .FoundFiles
// .Find(filter)
// .ToListAsync();
//}
//public async Task<IEnumerable<FoundFile>> GetFileBySize(long size) {
// FilterDefinition<FoundFile> filter = Builders<FoundFile>.Filter.ElemMatch<long>(p => p.Size, size);
// return await _context
// .FoundFiles
// .Find(filter)
// .ToListAsync();
//}
public async Task CreateFile(FoundFile foundFile) {
await _context.FoundFiles.InsertOneAsync(foundFile);
}
public async Task<bool> UpdateFile(FoundFile foundFile) {
var updateResult = await _context.FoundFiles.ReplaceOneAsync(
filter: g => g.Path == foundFile.Path, replacement: foundFile);
return updateResult.IsAcknowledged && updateResult.ModifiedCount > 0;
}
public async Task<bool> DeleteFile(string path) {
FilterDefinition<FoundFile> filter = Builders<FoundFile>.Filter.Eq(p => p.Path, path);
DeleteResult deleteResult = await _context
.FoundFiles
.DeleteOneAsync(filter);
return deleteResult.IsAcknowledged
&& deleteResult.DeletedCount > 0;
}
}
}
[FileSearchHostedService.cs]
using unite.radimaging.source.n2m2.Repositories;
using unite.radimaging.source.n2m2.Entities;
using unite.radimaging.source.n2m2.Data;
using Serilog;
using Microsoft.Extensions.Configuration;
using System;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.Hosting;
namespace unite.radimaging.source.n2m2.HostedServices.FileSearchHostedService {
public class FileSearchHostedService : BackgroundService {
private readonly IConfiguration _configuration;
//private IFoundFileRepository _repository;
public FileSearchHostedService(
//IFoundFileRepository repository,
IConfiguration configuration
) {
//_repository = repository ?? throw new ArgumentNullException(nameof(repository));
_configuration = configuration ?? throw new ArgumentNullException(nameof(configuration));
}
protected override async Task ExecuteAsync(CancellationToken cancellationToken) {
Log.Information("FileSearchHostedService: File searching started.");
cancellationToken.Register(() => Log.Information("Processing service stopped"));
//string checksum;
string _msg;
string _current_path;
FileInfo _current_fileinfo;
FoundFile _foundFile;
FoundFile _existingFile;
// Convert these two lines to Dependency Injections if possible
FoundFileContext FoundfileContext = new FoundFileContext(_configuration); // Normal instantiation, in leiu of injection
FoundFileRepository _repository = new FoundFileRepository(FoundfileContext); // Normal instantiation, in leiu of injection
string dir = _configuration.GetValue<string>("FileSearchSettings:SearchDir");
while (!cancellationToken.IsCancellationRequested) {
string[] files = Directory.GetFiles(dir);
foreach (string filename in files) {
_msg = $"'{filename}' ";
_current_fileinfo = new FileInfo(filename);
_current_path = Path.GetFullPath(Path.Combine(_current_fileinfo.Directory.ToString(), _current_fileinfo.Name));
_existingFile = await _repository.GetFileByPath (_current_path);
_foundFile = new FoundFile() {
Path = _current_path,
Size = _current_fileinfo.Length,
Mtime = _current_fileinfo.LastWriteTime,
Checksum = FileChecksum.getChecksum(filename)
};
if (_existingFile == null) {
_msg += $"does not exist in MongoDB. Creating new entry ";
// Creating a new entry in MongoDB should prolly be it's own class
// but until I figure out dependency injection for the repo, it creates more
// processing than its worth
try {
await _repository.CreateFile(_foundFile);
_msg += "(OK)";
}
catch (Exception e) {
_msg += $"(FAILED with '{e.Message}'";
}
}
else if (_foundFile.Equals(_existingFile)) {
_msg += "is already in the database. No further processing needed.";
}
else {
_msg += $"has changed since being added to MongoDB. Updating entry ";
try {
var _result = await _repository.UpdateFile(_foundFile);
if (!_result == true) {
throw new ApplicationException("FoundFileRepository.UpdateFile did not return 'true'. Unknown MongoDB error updating the document.");
}
_msg += "(OK)";
}
catch (Exception e) {
_msg += $"(FAILED with '{e.Message}')";
}
};
Log.Debug(_msg);
}
await Task.Delay(_configuration.GetValue<int>("FileSearchSettings:Delay"),cancellationToken);
}
}
}
}
FileSearchHostedService.cs... It will be searching, checking, and creating new MongoDB entries. It does a lot) and 3) How/If I can instantiate theFoundFileRepositoryas a Dependency Injection instead of a explicit new(). Thanks again! \$\endgroup\$3) How/Ifsince we only review working code and can't write new code. Other than that, excellent question. \$\endgroup\$_msg +=thing is deeply annoying. Just do aLog.Debugeach time, that way your code is clearer. 4) Decrease your nesting: at several points you have atryinside anif(orelse) inside aforeachinside awhile. \$\endgroup\$