From 45839ff0a9834890f6987f54046299f247c5df8d Mon Sep 17 00:00:00 2001 From: Nano Taboada <87288+nanotaboada@users.noreply.github.com> Date: Fri, 11 Apr 2025 11:26:49 -0300 Subject: [PATCH 1/3] refactor: use C# 12 primary constructors for injected deps --- .../Controllers/PlayerController.cs | 26 ++++---- .../Data/Repository.cs | 17 ++--- .../Services/PlayerService.cs | 66 ++++++++----------- 3 files changed, 45 insertions(+), 64 deletions(-) diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs index 2dd28eb..7cb1988 100644 --- a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs @@ -15,10 +15,6 @@ public class PlayerController( IValidator validator ) : ControllerBase { - private readonly IPlayerService _playerService = playerService; - private readonly ILogger _logger = logger; - private readonly IValidator validator = validator; - /* ------------------------------------------------------------------------- * HTTP POST * ---------------------------------------------------------------------- */ @@ -45,16 +41,16 @@ public async Task PostAsync([FromBody] PlayerRequestModel player) .Errors.Select(error => new { error.PropertyName, error.ErrorMessage }) .ToArray(); - _logger.LogWarning("POST validation failed: {@Errors}", errors); + logger.LogWarning("POST validation failed: {@Errors}", errors); return TypedResults.BadRequest(errors); } - if (await _playerService.RetrieveByIdAsync(player.Id) != null) + if (await playerService.RetrieveByIdAsync(player.Id) != null) { return TypedResults.Conflict(); } - var result = await _playerService.CreateAsync(player); + var result = await playerService.CreateAsync(player); return TypedResults.CreatedAtRoute( routeName: "GetById", @@ -77,7 +73,7 @@ public async Task PostAsync([FromBody] PlayerRequestModel player) [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task GetAsync() { - var players = await _playerService.RetrieveAsync(); + var players = await playerService.RetrieveAsync(); if (players.Count > 0) { @@ -100,7 +96,7 @@ public async Task GetAsync() [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task GetByIdAsync([FromRoute] long id) { - var player = await _playerService.RetrieveByIdAsync(id); + var player = await playerService.RetrieveByIdAsync(id); if (player != null) { @@ -123,7 +119,7 @@ public async Task GetByIdAsync([FromRoute] long id) [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task GetBySquadNumberAsync([FromRoute] int squadNumber) { - var player = await _playerService.RetrieveBySquadNumberAsync(squadNumber); + var player = await playerService.RetrieveBySquadNumberAsync(squadNumber); if (player != null) { @@ -162,16 +158,16 @@ public async Task PutAsync([FromRoute] long id, [FromBody] PlayerReques .Errors.Select(error => new { error.PropertyName, error.ErrorMessage }) .ToArray(); - _logger.LogWarning("PUT /players/{Id} validation failed: {@Errors}", id, errors); + logger.LogWarning("PUT /players/{Id} validation failed: {@Errors}", id, errors); return TypedResults.BadRequest(errors); } - if (await _playerService.RetrieveByIdAsync(id) == null) + if (await playerService.RetrieveByIdAsync(id) == null) { return TypedResults.NotFound(); } - await _playerService.UpdateAsync(player); + await playerService.UpdateAsync(player); return TypedResults.NoContent(); } @@ -191,13 +187,13 @@ public async Task PutAsync([FromRoute] long id, [FromBody] PlayerReques [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task DeleteAsync([FromRoute] long id) { - if (await _playerService.RetrieveByIdAsync(id) == null) + if (await playerService.RetrieveByIdAsync(id) == null) { return TypedResults.NotFound(); } else { - await _playerService.DeleteAsync(id); + await playerService.DeleteAsync(id); return TypedResults.NoContent(); } diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Data/Repository.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Data/Repository.cs index ef698af..e77f7f9 100644 --- a/src/Dotnet.Samples.AspNetCore.WebApi/Data/Repository.cs +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Data/Repository.cs @@ -2,22 +2,15 @@ namespace Dotnet.Samples.AspNetCore.WebApi.Data; -public class Repository : IRepository +public class Repository(DbContext dbContext) : IRepository where T : class { - protected readonly DbContext _dbContext; - protected readonly DbSet _dbSet; - - public Repository(DbContext dbContext) - { - _dbContext = dbContext; - _dbSet = _dbContext.Set(); - } + protected readonly DbSet _dbSet = dbContext.Set(); public async Task AddAsync(T entity) { await _dbSet.AddAsync(entity); - await _dbContext.SaveChangesAsync(); + await dbContext.SaveChangesAsync(); } public async Task> GetAllAsync() => await _dbSet.AsNoTracking().ToListAsync(); @@ -27,7 +20,7 @@ public async Task AddAsync(T entity) public async Task UpdateAsync(T entity) { _dbSet.Update(entity); - await _dbContext.SaveChangesAsync(); + await dbContext.SaveChangesAsync(); } public async Task RemoveAsync(long id) @@ -36,7 +29,7 @@ public async Task RemoveAsync(long id) if (entity != null) { _dbSet.Remove(entity); - await _dbContext.SaveChangesAsync(); + await dbContext.SaveChangesAsync(); } } } diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Services/PlayerService.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Services/PlayerService.cs index d5a6b79..b3be33f 100644 --- a/src/Dotnet.Samples.AspNetCore.WebApi/Services/PlayerService.cs +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Services/PlayerService.cs @@ -16,26 +16,18 @@ IMapper mapper private static readonly string AspNetCore_Environment = "ASPNETCORE_ENVIRONMENT"; private static readonly string Development = "Development"; - private readonly IPlayerRepository _playerRepository = playerRepository; - private readonly ILogger _logger = logger; - private readonly IMemoryCache _memoryCache = memoryCache; - private readonly IMapper _mapper = mapper; - /* ------------------------------------------------------------------------- * Create * ---------------------------------------------------------------------- */ public async Task CreateAsync(PlayerRequestModel playerRequestModel) { - var player = _mapper.Map(playerRequestModel); - await _playerRepository.AddAsync(player); - _logger.LogInformation("Player added to Repository: {Player}", player); - _memoryCache.Remove(CacheKey_RetrieveAsync); - _logger.LogInformation( - "Removed objects from Cache with Key: {Key}", - CacheKey_RetrieveAsync - ); - return _mapper.Map(player); + var player = mapper.Map(playerRequestModel); + await playerRepository.AddAsync(player); + logger.LogInformation("Player added to Repository: {Player}", player); + memoryCache.Remove(CacheKey_RetrieveAsync); + logger.LogInformation("Removed objects from Cache with Key: {Key}", CacheKey_RetrieveAsync); + return mapper.Map(player); } /* ------------------------------------------------------------------------- @@ -44,9 +36,9 @@ public async Task CreateAsync(PlayerRequestModel playerRequ public async Task> RetrieveAsync() { - if (_memoryCache.TryGetValue(CacheKey_RetrieveAsync, out List? cached)) + if (memoryCache.TryGetValue(CacheKey_RetrieveAsync, out List? cached)) { - _logger.LogInformation("Players retrieved from Cache"); + logger.LogInformation("Players retrieved from Cache"); return cached!; } else @@ -58,12 +50,12 @@ public async Task> RetrieveAsync() await SimulateRepositoryDelayAsync(); } - var players = await _playerRepository.GetAllAsync(); - _logger.LogInformation("Players retrieved from Repository"); - var playerResponseModels = _mapper.Map>(players); - using (var cacheEntry = _memoryCache.CreateEntry(CacheKey_RetrieveAsync)) + var players = await playerRepository.GetAllAsync(); + logger.LogInformation("Players retrieved from Repository"); + var playerResponseModels = mapper.Map>(players); + using (var cacheEntry = memoryCache.CreateEntry(CacheKey_RetrieveAsync)) { - _logger.LogInformation( + logger.LogInformation( "{Count} entries created in Cache with key: {Key}", playerResponseModels.Count, CacheKey_RetrieveAsync @@ -78,14 +70,14 @@ public async Task> RetrieveAsync() public async Task RetrieveByIdAsync(long id) { - var player = await _playerRepository.FindByIdAsync(id); - return player is not null ? _mapper.Map(player) : null; + var player = await playerRepository.FindByIdAsync(id); + return player is not null ? mapper.Map(player) : null; } public async Task RetrieveBySquadNumberAsync(int squadNumber) { - var player = await _playerRepository.FindBySquadNumberAsync(squadNumber); - return player is not null ? _mapper.Map(player) : null; + var player = await playerRepository.FindBySquadNumberAsync(squadNumber); + return player is not null ? mapper.Map(player) : null; } /* ------------------------------------------------------------------------- @@ -94,13 +86,13 @@ public async Task> RetrieveAsync() public async Task UpdateAsync(PlayerRequestModel playerRequestModel) { - if (await _playerRepository.FindByIdAsync(playerRequestModel.Id) is Player player) + if (await playerRepository.FindByIdAsync(playerRequestModel.Id) is Player player) { - _mapper.Map(playerRequestModel, player); - await _playerRepository.UpdateAsync(player); - _logger.LogInformation("Player updated in Repository: {Player}", player); - _memoryCache.Remove(CacheKey_RetrieveAsync); - _logger.LogInformation( + mapper.Map(playerRequestModel, player); + await playerRepository.UpdateAsync(player); + logger.LogInformation("Player updated in Repository: {Player}", player); + memoryCache.Remove(CacheKey_RetrieveAsync); + logger.LogInformation( "Removed objects from Cache with Key: {Key}", CacheKey_RetrieveAsync ); @@ -113,12 +105,12 @@ public async Task UpdateAsync(PlayerRequestModel playerRequestModel) public async Task DeleteAsync(long id) { - if (await _playerRepository.FindByIdAsync(id) is not null) + if (await playerRepository.FindByIdAsync(id) is not null) { - await _playerRepository.RemoveAsync(id); - _logger.LogInformation("Player with Id {Id} removed from Repository", id); - _memoryCache.Remove(CacheKey_RetrieveAsync); - _logger.LogInformation( + await playerRepository.RemoveAsync(id); + logger.LogInformation("Player with Id {Id} removed from Repository", id); + memoryCache.Remove(CacheKey_RetrieveAsync); + logger.LogInformation( "Removed objects from Cache with Key: {Key}", CacheKey_RetrieveAsync ); @@ -147,7 +139,7 @@ private static MemoryCacheEntryOptions GetMemoryCacheEntryOptions() private async Task SimulateRepositoryDelayAsync() { var milliseconds = new Random().Next(2600, 4200); - _logger.LogInformation( + logger.LogInformation( "Simulating a random delay of {Milliseconds} milliseconds...", milliseconds ); From 4720a4e504c16284f332bb954665e01eb0c604a8 Mon Sep 17 00:00:00 2001 From: Nano Taboada <87288+nanotaboada@users.noreply.github.com> Date: Fri, 11 Apr 2025 15:04:52 -0300 Subject: [PATCH 2/3] refactor(services): move MemoryCacheEntryOptions init to static field --- .../Services/PlayerService.cs | 43 +++++++++++-------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Services/PlayerService.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Services/PlayerService.cs index b3be33f..c8ef34c 100644 --- a/src/Dotnet.Samples.AspNetCore.WebApi/Services/PlayerService.cs +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Services/PlayerService.cs @@ -12,8 +12,33 @@ public class PlayerService( IMapper mapper ) : IPlayerService { + /// + /// Creates a MemoryCacheEntryOptions instance with Normal priority, + /// SlidingExpiration of 10 minutes and AbsoluteExpiration of 1 hour. + /// + private static readonly MemoryCacheEntryOptions CacheEntryOptions = + new MemoryCacheEntryOptions() + .SetPriority(CacheItemPriority.Normal) + .SetSlidingExpiration(TimeSpan.FromMinutes(10)) + .SetAbsoluteExpiration(TimeSpan.FromHours(1)); + + /// + /// The key used to store the list of Players in the cache. + /// private static readonly string CacheKey_RetrieveAsync = nameof(RetrieveAsync); + + /// + /// The key used to store the environment variable for ASP.NET Core. + ///
+ /// + /// Use multiple environments in ASP.NET Core + /// + /// private static readonly string AspNetCore_Environment = "ASPNETCORE_ENVIRONMENT"; + + /// + /// The value used to check if the environment is Development. + /// private static readonly string Development = "Development"; /* ------------------------------------------------------------------------- @@ -43,13 +68,10 @@ public async Task> RetrieveAsync() } else { - // Use multiple environments in ASP.NET Core - // https://learn.microsoft.com/en-us/aspnet/core/fundamentals/environments?view=aspnetcore-8.0 if (Environment.GetEnvironmentVariable(AspNetCore_Environment) == Development) { await SimulateRepositoryDelayAsync(); } - var players = await playerRepository.GetAllAsync(); logger.LogInformation("Players retrieved from Repository"); var playerResponseModels = mapper.Map>(players); @@ -62,7 +84,7 @@ public async Task> RetrieveAsync() ); cacheEntry.SetSize(playerResponseModels.Count); cacheEntry.Value = playerResponseModels; - cacheEntry.SetOptions(GetMemoryCacheEntryOptions()); + cacheEntry.SetOptions(CacheEntryOptions); } return playerResponseModels; } @@ -117,19 +139,6 @@ public async Task DeleteAsync(long id) } } - /// - /// Creates a MemoryCacheEntryOptions instance with Normal priority, - /// SlidingExpiration of 10 minutes and AbsoluteExpiration of 1 hour. - /// - /// A MemoryCacheEntryOptions instance with the specified options. - private static MemoryCacheEntryOptions GetMemoryCacheEntryOptions() - { - return new MemoryCacheEntryOptions() - .SetPriority(CacheItemPriority.Normal) - .SetSlidingExpiration(TimeSpan.FromMinutes(10)) - .SetAbsoluteExpiration(TimeSpan.FromHours(1)); - } - /// /// Simulates a delay in the repository call to mimic a long-running operation. /// This is only used in the Development environment to simulate a delay From 4af450279266362246762a9b21bced5215230e66 Mon Sep 17 00:00:00 2001 From: Nano Taboada <87288+nanotaboada@users.noreply.github.com> Date: Fri, 11 Apr 2025 15:05:03 -0300 Subject: [PATCH 3/3] chore(controllers): enrich log events --- .../Controllers/PlayerController.cs | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs index 7cb1988..5c731b6 100644 --- a/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs +++ b/src/Dotnet.Samples.AspNetCore.WebApi/Controllers/PlayerController.cs @@ -41,17 +41,22 @@ public async Task PostAsync([FromBody] PlayerRequestModel player) .Errors.Select(error => new { error.PropertyName, error.ErrorMessage }) .ToArray(); - logger.LogWarning("POST validation failed: {@Errors}", errors); + logger.LogWarning("POST /players validation failed: {@Errors}", errors); return TypedResults.BadRequest(errors); } if (await playerService.RetrieveByIdAsync(player.Id) != null) { + logger.LogWarning( + "POST /players failed: Player with ID {Id} already exists", + player.Id + ); return TypedResults.Conflict(); } var result = await playerService.CreateAsync(player); + logger.LogInformation("POST /players created: {@Player}", result); return TypedResults.CreatedAtRoute( routeName: "GetById", routeValues: new { id = result.Id }, @@ -77,10 +82,12 @@ public async Task GetAsync() if (players.Count > 0) { + logger.LogInformation("GET /players retrieved"); return TypedResults.Ok(players); } else { + logger.LogWarning("GET /players not found"); return TypedResults.NotFound(); } } @@ -97,13 +104,14 @@ public async Task GetAsync() public async Task GetByIdAsync([FromRoute] long id) { var player = await playerService.RetrieveByIdAsync(id); - if (player != null) { + logger.LogInformation("GET /players/{Id} retrieved: {@Player}", id, player); return TypedResults.Ok(player); } else { + logger.LogWarning("GET /players/{Id} not found", id); return TypedResults.NotFound(); } } @@ -120,13 +128,18 @@ public async Task GetByIdAsync([FromRoute] long id) public async Task GetBySquadNumberAsync([FromRoute] int squadNumber) { var player = await playerService.RetrieveBySquadNumberAsync(squadNumber); - if (player != null) { + logger.LogInformation( + "GET /players/squad/{SquadNumber} retrieved: {@Player}", + squadNumber, + player + ); return TypedResults.Ok(player); } else { + logger.LogWarning("GET /players/squad/{SquadNumber} not found", squadNumber); return TypedResults.NotFound(); } } @@ -151,7 +164,6 @@ public async Task GetBySquadNumberAsync([FromRoute] int squadNumber) public async Task PutAsync([FromRoute] long id, [FromBody] PlayerRequestModel player) { var validation = await validator.ValidateAsync(player); - if (!validation.IsValid) { var errors = validation @@ -161,14 +173,13 @@ public async Task PutAsync([FromRoute] long id, [FromBody] PlayerReques logger.LogWarning("PUT /players/{Id} validation failed: {@Errors}", id, errors); return TypedResults.BadRequest(errors); } - if (await playerService.RetrieveByIdAsync(id) == null) { + logger.LogWarning("PUT /players/{Id} not found", id); return TypedResults.NotFound(); } - await playerService.UpdateAsync(player); - + logger.LogInformation("PUT /players/{Id} updated: {@Player}", id, player); return TypedResults.NoContent(); } @@ -189,12 +200,13 @@ public async Task DeleteAsync([FromRoute] long id) { if (await playerService.RetrieveByIdAsync(id) == null) { + logger.LogWarning("DELETE /players/{Id} not found", id); return TypedResults.NotFound(); } else { await playerService.DeleteAsync(id); - + logger.LogInformation("DELETE /players/{Id} deleted", id); return TypedResults.NoContent(); } }