From ec057e5bcf9a0a27e49f604747083762055d14f0 Mon Sep 17 00:00:00 2001 From: Nadir Badnjevic Date: Mon, 16 Dec 2024 09:42:23 +0100 Subject: [PATCH 1/6] feat: refactor validation behavior to use ErrorOr --- Directory.Packages.props | 2 +- requests/TodoItems/DeleteTodoItem.http | 9 ++++ requests/TodoItems/UpdateTodoItemDetails.http | 14 ++++++ requests/TodoLists/CreateTodoList.http | 18 +++++++- requests/TodoLists/DeleteTodoList.http | 9 ++++ src/Application/Common/ApiControllerBase.cs | 42 +++++++++++++++++ .../Behaviours/UnhandledExceptionBehaviour.cs | 32 ------------- .../Common/Behaviours/ValidationBehaviour.cs | 46 ++++++++++--------- src/Application/ConfigureServices.cs | 7 ++- .../Features/TodoItems/CreateTodoItem.cs | 18 +++++--- .../Features/TodoItems/DeleteTodoItem.cs | 35 +++++++++----- .../TodoItems/GetTodoItemsWithPagination.cs | 22 ++++++--- .../Features/TodoItems/UpdateTodoItem.cs | 34 +++++++++----- .../TodoItems/UpdateTodoItemDetail.cs | 33 +++++++++---- .../Features/TodoLists/CreateTodoList.cs | 18 +++++--- .../Features/TodoLists/DeleteTodoList.cs | 29 +++++++----- 16 files changed, 246 insertions(+), 122 deletions(-) create mode 100644 requests/TodoItems/DeleteTodoItem.http create mode 100644 requests/TodoItems/UpdateTodoItemDetails.http create mode 100644 requests/TodoLists/DeleteTodoList.http delete mode 100644 src/Application/Common/Behaviours/UnhandledExceptionBehaviour.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index 8a3520a..f2285ff 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -13,7 +13,7 @@ - + diff --git a/requests/TodoItems/DeleteTodoItem.http b/requests/TodoItems/DeleteTodoItem.http new file mode 100644 index 0000000..3f07884 --- /dev/null +++ b/requests/TodoItems/DeleteTodoItem.http @@ -0,0 +1,9 @@ +@host = https://localhost:7098 + + +### Delete Todo itemId = 10 with wrong itemId. Check for 404 Not Found +DELETE {{host}}/api/todo-items/10 + + +### Delete Todo itemId = 1, should return 204 No Content +DELETE {{host}}/api/todo-items/1 diff --git a/requests/TodoItems/UpdateTodoItemDetails.http b/requests/TodoItems/UpdateTodoItemDetails.http new file mode 100644 index 0000000..524f1bc --- /dev/null +++ b/requests/TodoItems/UpdateTodoItemDetails.http @@ -0,0 +1,14 @@ +@host = https://localhost:7098 +@itemId = 4 +@listId = 1 + +### Update Todo itemId = 1 with PriorityLevel = 1 and Note = "Updated Todo Item 1" +PUT {{host}}/api/todo-items/UpdateItemDetails?id={{itemId}} +Content-Type: application/json + +{ + "id": {{itemId}}, + "listId": {{listId}}, + "priorityLevel": 1, + "note": "Updated Todo Item 1" +} \ No newline at end of file diff --git a/requests/TodoLists/CreateTodoList.http b/requests/TodoLists/CreateTodoList.http index acebe35..5dc691c 100644 --- a/requests/TodoLists/CreateTodoList.http +++ b/requests/TodoLists/CreateTodoList.http @@ -5,5 +5,21 @@ POST {{host}}/api/todo-lists Content-Type: application/json { - "title": "List 2" + "title": "List 1" +} + +### Create Todo List item, validate that it handles empty title +POST {{host}}/api/todo-lists +Content-Type: application/json + +{ + "title": "" +} + +### Create Todo List item, validate that it duplicate title +POST {{host}}/api/todo-lists +Content-Type: application/json + +{ + "title": "List 1" } \ No newline at end of file diff --git a/requests/TodoLists/DeleteTodoList.http b/requests/TodoLists/DeleteTodoList.http new file mode 100644 index 0000000..d385d1d --- /dev/null +++ b/requests/TodoLists/DeleteTodoList.http @@ -0,0 +1,9 @@ +@host = https://localhost:7098 + + +### Delete Todo itemId = 10 with wrong itemId. Check for 404 Not Found +DELETE {{host}}/api/todo-lists/10 + + +### Delete Todo itemId = 1, should return 204 No Content +DELETE {{host}}/api/todo-lists/1 diff --git a/src/Application/Common/ApiControllerBase.cs b/src/Application/Common/ApiControllerBase.cs index 0812af5..b1a525b 100644 --- a/src/Application/Common/ApiControllerBase.cs +++ b/src/Application/Common/ApiControllerBase.cs @@ -1,6 +1,10 @@ +using ErrorOr; + using MediatR; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.Extensions.DependencyInjection; namespace VerticalSliceArchitecture.Application.Common; @@ -12,4 +16,42 @@ public abstract class ApiControllerBase : ControllerBase private ISender? _mediator; protected ISender Mediator => _mediator ??= HttpContext.RequestServices.GetService()!; + + protected ActionResult Problem(List errors) + { + if (errors.Count is 0) + { + return Problem(); + } + + if (errors.All(error => error.Type == ErrorType.Validation)) + { + return ValidationProblem(errors); + } + + return Problem(errors[0]); + } + + private ObjectResult Problem(Error error) + { + var statusCode = error.Type switch + { + ErrorType.Conflict => StatusCodes.Status409Conflict, + ErrorType.Validation => StatusCodes.Status400BadRequest, + ErrorType.NotFound => StatusCodes.Status404NotFound, + ErrorType.Unauthorized => StatusCodes.Status403Forbidden, + _ => StatusCodes.Status500InternalServerError, + }; + + return Problem(statusCode: statusCode, title: error.Description); + } + + private ActionResult ValidationProblem(List errors) + { + var modelStateDictionary = new ModelStateDictionary(); + + errors.ForEach(error => modelStateDictionary.AddModelError(error.Code, error.Description)); + + return ValidationProblem(modelStateDictionary); + } } \ No newline at end of file diff --git a/src/Application/Common/Behaviours/UnhandledExceptionBehaviour.cs b/src/Application/Common/Behaviours/UnhandledExceptionBehaviour.cs deleted file mode 100644 index d5870d3..0000000 --- a/src/Application/Common/Behaviours/UnhandledExceptionBehaviour.cs +++ /dev/null @@ -1,32 +0,0 @@ -using MediatR; - -using Microsoft.Extensions.Logging; - -namespace VerticalSliceArchitecture.Application.Common.Behaviours; - -public class UnhandledExceptionBehaviour : IPipelineBehavior - where TRequest : IRequest -{ - private readonly ILogger _logger; - - public UnhandledExceptionBehaviour(ILogger logger) - { - _logger = logger; - } - - public async Task Handle(TRequest request, RequestHandlerDelegate next, CancellationToken cancellationToken) - { - try - { - return await next(); - } - catch (Exception ex) - { - var requestName = typeof(TRequest).Name; - - _logger.LogError(ex, "VerticalSlice Request: Unhandled Exception for Request {Name} {@Request}", requestName, request); - - throw; - } - } -} \ No newline at end of file diff --git a/src/Application/Common/Behaviours/ValidationBehaviour.cs b/src/Application/Common/Behaviours/ValidationBehaviour.cs index af70609..26e4165 100644 --- a/src/Application/Common/Behaviours/ValidationBehaviour.cs +++ b/src/Application/Common/Behaviours/ValidationBehaviour.cs @@ -1,36 +1,40 @@ -using FluentValidation; +using ErrorOr; -using MediatR; +using FluentValidation; -using ValidationException = VerticalSliceArchitecture.Application.Common.Exceptions.ValidationException; +using MediatR; namespace VerticalSliceArchitecture.Application.Common.Behaviours; -public class ValidationBehaviour : IPipelineBehavior - where TRequest : IRequest +public class ValidationBehavior(IValidator? validator = null) + : IPipelineBehavior + where TRequest : IRequest + where TResponse : IErrorOr { - private readonly IEnumerable> _validators; - - public ValidationBehaviour(IEnumerable> validators) - { - _validators = validators; - } + private readonly IValidator? _validator = validator; - public async Task Handle(TRequest request, RequestHandlerDelegate next, CancellationToken cancellationToken) + public async Task Handle( + TRequest request, + RequestHandlerDelegate next, + CancellationToken cancellationToken) { - if (_validators.Any()) + if (_validator is null) { - var context = new ValidationContext(request); + return await next(); + } - var validationResults = await Task.WhenAll(_validators.Select(v => v.ValidateAsync(context, cancellationToken))); - var failures = validationResults.SelectMany(r => r.Errors).Where(f => f != null).ToList(); + var validationResult = await _validator.ValidateAsync(request, cancellationToken); - if (failures.Count != 0) - { - throw new ValidationException(failures); - } + if (validationResult.IsValid) + { + return await next(); } - return await next(); + var errors = validationResult.Errors + .ConvertAll(error => Error.Validation( + code: error.PropertyName, + description: error.ErrorMessage)); + + return (dynamic)errors; } } \ No newline at end of file diff --git a/src/Application/ConfigureServices.cs b/src/Application/ConfigureServices.cs index cda66c7..1fbc9d1 100644 --- a/src/Application/ConfigureServices.cs +++ b/src/Application/ConfigureServices.cs @@ -16,18 +16,17 @@ public static class DependencyInjection { public static IServiceCollection AddApplication(this IServiceCollection services) { - services.AddValidatorsFromAssembly(typeof(DependencyInjection).Assembly); - services.AddMediatR(options => { options.RegisterServicesFromAssembly(typeof(DependencyInjection).Assembly); options.AddOpenBehavior(typeof(AuthorizationBehaviour<,>)); - options.AddOpenBehavior(typeof(ValidationBehaviour<,>)); options.AddOpenBehavior(typeof(PerformanceBehaviour<,>)); - options.AddOpenBehavior(typeof(UnhandledExceptionBehaviour<,>)); + options.AddOpenBehavior(typeof(ValidationBehavior<,>)); }); + services.AddValidatorsFromAssembly(typeof(DependencyInjection).Assembly, includeInternalTypes: true); + return services; } diff --git a/src/Application/Features/TodoItems/CreateTodoItem.cs b/src/Application/Features/TodoItems/CreateTodoItem.cs index 6238a0f..2193525 100644 --- a/src/Application/Features/TodoItems/CreateTodoItem.cs +++ b/src/Application/Features/TodoItems/CreateTodoItem.cs @@ -1,4 +1,6 @@ -using FluentValidation; +using ErrorOr; + +using FluentValidation; using MediatR; @@ -13,13 +15,17 @@ namespace VerticalSliceArchitecture.Application.Features.TodoItems; public class CreateTodoItemController : ApiControllerBase { [HttpPost("/api/todo-items")] - public async Task> Create(CreateTodoItemCommand command) + public async Task Create(CreateTodoItemCommand command) { - return await Mediator.Send(command); + var result = await Mediator.Send(command); + + return result.Match( + id => Ok(id), + Problem); } } -public record CreateTodoItemCommand(int ListId, string? Title) : IRequest; +public record CreateTodoItemCommand(int ListId, string? Title) : IRequest>; internal sealed class CreateTodoItemCommandValidator : AbstractValidator { @@ -31,11 +37,11 @@ public CreateTodoItemCommandValidator() } } -internal sealed class CreateTodoItemCommandHandler(ApplicationDbContext context) : IRequestHandler +internal sealed class CreateTodoItemCommandHandler(ApplicationDbContext context) : IRequestHandler> { private readonly ApplicationDbContext _context = context; - public async Task Handle(CreateTodoItemCommand request, CancellationToken cancellationToken) + public async Task> Handle(CreateTodoItemCommand request, CancellationToken cancellationToken) { var entity = new TodoItem { diff --git a/src/Application/Features/TodoItems/DeleteTodoItem.cs b/src/Application/Features/TodoItems/DeleteTodoItem.cs index b9acfad..914331c 100644 --- a/src/Application/Features/TodoItems/DeleteTodoItem.cs +++ b/src/Application/Features/TodoItems/DeleteTodoItem.cs @@ -1,9 +1,10 @@ -using MediatR; +using ErrorOr; + +using MediatR; using Microsoft.AspNetCore.Mvc; using VerticalSliceArchitecture.Application.Common; -using VerticalSliceArchitecture.Application.Common.Exceptions; using VerticalSliceArchitecture.Application.Domain.Todos; using VerticalSliceArchitecture.Application.Infrastructure.Persistence; @@ -12,28 +13,38 @@ namespace VerticalSliceArchitecture.Application.Features.TodoItems; public class DeleteTodoItemController : ApiControllerBase { [HttpDelete("/api/todo-items/{id}")] - public async Task Delete(int id) + public async Task Delete(int id) { - await Mediator.Send(new DeleteTodoItemCommand(id)); + var result = await Mediator.Send(new DeleteTodoItemCommand(id)); - return NoContent(); + return result.Match( + _ => NoContent(), + Problem); } } -public record DeleteTodoItemCommand(int Id) : IRequest; +public record DeleteTodoItemCommand(int Id) : IRequest>; -internal sealed class DeleteTodoItemCommandHandler(ApplicationDbContext context) : IRequestHandler +internal sealed class DeleteTodoItemCommandHandler(ApplicationDbContext context) : IRequestHandler> { private readonly ApplicationDbContext _context = context; - public async Task Handle(DeleteTodoItemCommand request, CancellationToken cancellationToken) + public async Task> Handle(DeleteTodoItemCommand request, CancellationToken cancellationToken) { - var entity = await _context.TodoItems - .FindAsync(new object[] { request.Id }, cancellationToken) ?? throw new NotFoundException(nameof(TodoItem), request.Id); - _context.TodoItems.Remove(entity); + var todoItem = await _context.TodoItems + .FindAsync([request.Id], cancellationToken); + + if (todoItem is null) + { + return Error.NotFound(description: "Todo item not found."); + } - entity.DomainEvents.Add(new TodoItemDeletedEvent(entity)); + _context.TodoItems.Remove(todoItem); + + todoItem.DomainEvents.Add(new TodoItemDeletedEvent(todoItem)); await _context.SaveChangesAsync(cancellationToken); + + return Result.Success; } } \ No newline at end of file diff --git a/src/Application/Features/TodoItems/GetTodoItemsWithPagination.cs b/src/Application/Features/TodoItems/GetTodoItemsWithPagination.cs index a941231..f0ecfdd 100644 --- a/src/Application/Features/TodoItems/GetTodoItemsWithPagination.cs +++ b/src/Application/Features/TodoItems/GetTodoItemsWithPagination.cs @@ -1,4 +1,6 @@ -using FluentValidation; +using ErrorOr; + +using FluentValidation; using MediatR; @@ -15,15 +17,19 @@ namespace VerticalSliceArchitecture.Application.Features.TodoItems; public class GetTodoItemsWithPaginationController : ApiControllerBase { [HttpGet("/api/todo-items")] - public Task> GetTodoItemsWithPagination([FromQuery] GetTodoItemsWithPaginationQuery query) + public async Task GetTodoItemsWithPagination([FromQuery] GetTodoItemsWithPaginationQuery query) { - return Mediator.Send(query); + var result = await Mediator.Send(query); + + return result.Match( + Ok, + Problem); } } public record TodoItemBriefResponse(int Id, int ListId, string? Title, bool Done); -public record GetTodoItemsWithPaginationQuery(int ListId, int PageNumber = 1, int PageSize = 10) : IRequest>; +public record GetTodoItemsWithPaginationQuery(int ListId, int PageNumber = 1, int PageSize = 10) : IRequest>>; internal sealed class GetTodoItemsWithPaginationQueryValidator : AbstractValidator { @@ -40,17 +46,19 @@ public GetTodoItemsWithPaginationQueryValidator() } } -internal sealed class GetTodoItemsWithPaginationQueryHandler(ApplicationDbContext context) : IRequestHandler> +internal sealed class GetTodoItemsWithPaginationQueryHandler(ApplicationDbContext context) : IRequestHandler>> { private readonly ApplicationDbContext _context = context; - public Task> Handle(GetTodoItemsWithPaginationQuery request, CancellationToken cancellationToken) + public async Task>> Handle(GetTodoItemsWithPaginationQuery request, CancellationToken cancellationToken) { - return _context.TodoItems + var paginatedList = await _context.TodoItems .Where(item => item.ListId == request.ListId) .OrderBy(item => item.Title) .Select(item => ToDto(item)) .PaginatedListAsync(request.PageNumber, request.PageSize); + + return paginatedList; } private static TodoItemBriefResponse ToDto(TodoItem todoItem) => diff --git a/src/Application/Features/TodoItems/UpdateTodoItem.cs b/src/Application/Features/TodoItems/UpdateTodoItem.cs index d9585bb..15ab8c8 100644 --- a/src/Application/Features/TodoItems/UpdateTodoItem.cs +++ b/src/Application/Features/TodoItems/UpdateTodoItem.cs @@ -1,12 +1,13 @@ -using FluentValidation; +using ErrorOr; + +using FluentValidation; using MediatR; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using VerticalSliceArchitecture.Application.Common; -using VerticalSliceArchitecture.Application.Common.Exceptions; -using VerticalSliceArchitecture.Application.Domain.Todos; using VerticalSliceArchitecture.Application.Infrastructure.Persistence; namespace VerticalSliceArchitecture.Application.Features.TodoItems; @@ -14,20 +15,24 @@ namespace VerticalSliceArchitecture.Application.Features.TodoItems; public class TodoItemsController : ApiControllerBase { [HttpPut("/api/todo-items/{id}")] - public async Task Update(int id, UpdateTodoItemCommand command) + public async Task Update(int id, UpdateTodoItemCommand command) { if (id != command.Id) { - return BadRequest(); + return Problem( + statusCode: StatusCodes.Status400BadRequest, + detail: "Not matching ids"); } - await Mediator.Send(command); + var result = await Mediator.Send(command); - return NoContent(); + return result.Match( + _ => NoContent(), + Problem); } } -public record UpdateTodoItemCommand(int Id, string? Title, bool Done) : IRequest; +public record UpdateTodoItemCommand(int Id, string? Title, bool Done) : IRequest>; internal sealed class UpdateTodoItemCommandValidator : AbstractValidator { @@ -39,18 +44,25 @@ public UpdateTodoItemCommandValidator() } } -internal sealed class UpdateTodoItemCommandHandler(ApplicationDbContext context) : IRequestHandler +internal sealed class UpdateTodoItemCommandHandler(ApplicationDbContext context) : IRequestHandler> { private readonly ApplicationDbContext _context = context; - public async Task Handle(UpdateTodoItemCommand request, CancellationToken cancellationToken) + public async Task> Handle(UpdateTodoItemCommand request, CancellationToken cancellationToken) { var todoItem = await _context.TodoItems - .FindAsync(new object[] { request.Id }, cancellationToken) ?? throw new NotFoundException(nameof(TodoItem), request.Id); + .FindAsync([request.Id], cancellationToken); + + if (todoItem is null) + { + return Error.NotFound(description: "Todo item not found."); + } todoItem.Title = request.Title; todoItem.Done = request.Done; await _context.SaveChangesAsync(cancellationToken); + + return Result.Success; } } \ No newline at end of file diff --git a/src/Application/Features/TodoItems/UpdateTodoItemDetail.cs b/src/Application/Features/TodoItems/UpdateTodoItemDetail.cs index 6168f3a..128be0d 100644 --- a/src/Application/Features/TodoItems/UpdateTodoItemDetail.cs +++ b/src/Application/Features/TodoItems/UpdateTodoItemDetail.cs @@ -1,9 +1,11 @@ -using MediatR; +using ErrorOr; +using MediatR; + +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using VerticalSliceArchitecture.Application.Common; -using VerticalSliceArchitecture.Application.Common.Exceptions; using VerticalSliceArchitecture.Application.Domain.Todos; using VerticalSliceArchitecture.Application.Infrastructure.Persistence; @@ -12,34 +14,45 @@ namespace VerticalSliceArchitecture.Application.Features.TodoItems; public class UpdateTodoItemDetailController : ApiControllerBase { [HttpPut("/api/todo-items/[action]")] - public async Task UpdateItemDetails(int id, UpdateTodoItemDetailCommand command) + public async Task UpdateItemDetails(int id, UpdateTodoItemDetailCommand command) { if (id != command.Id) { - return BadRequest(); + return Problem( + statusCode: StatusCodes.Status400BadRequest, + detail: "Not matching ids"); } - await Mediator.Send(command); + var result = await Mediator.Send(command); - return NoContent(); + return result.Match( + _ => NoContent(), + Problem); } } -public record UpdateTodoItemDetailCommand(int Id, int ListId, PriorityLevel Priority, string? Note) : IRequest; +public record UpdateTodoItemDetailCommand(int Id, int ListId, PriorityLevel Priority, string? Note) : IRequest>; -internal sealed class UpdateTodoItemDetailCommandHandler(ApplicationDbContext context) : IRequestHandler +internal sealed class UpdateTodoItemDetailCommandHandler(ApplicationDbContext context) : IRequestHandler> { private readonly ApplicationDbContext _context = context; - public async Task Handle(UpdateTodoItemDetailCommand request, CancellationToken cancellationToken) + public async Task> Handle(UpdateTodoItemDetailCommand request, CancellationToken cancellationToken) { var todoItem = await _context.TodoItems - .FindAsync(new object[] { request.Id }, cancellationToken) ?? throw new NotFoundException(nameof(TodoItem), request.Id); + .FindAsync([request.Id], cancellationToken); + + if (todoItem is null) + { + return Error.NotFound(description: "Todo item not found."); + } todoItem.ListId = request.ListId; todoItem.Priority = request.Priority; todoItem.Note = request.Note; await _context.SaveChangesAsync(cancellationToken); + + return Result.Success; } } \ No newline at end of file diff --git a/src/Application/Features/TodoLists/CreateTodoList.cs b/src/Application/Features/TodoLists/CreateTodoList.cs index 2f456e0..56fd838 100644 --- a/src/Application/Features/TodoLists/CreateTodoList.cs +++ b/src/Application/Features/TodoLists/CreateTodoList.cs @@ -1,4 +1,6 @@ -using FluentValidation; +using ErrorOr; + +using FluentValidation; using MediatR; @@ -14,13 +16,17 @@ namespace VerticalSliceArchitecture.Application.Features.TodoLists; public class CreateTodoListController : ApiControllerBase { [HttpPost("/api/todo-lists")] - public async Task> Create(CreateTodoListCommand command) + public async Task Create(CreateTodoListCommand command) { - return await Mediator.Send(command); + var result = await Mediator.Send(command); + + return result.Match( + id => Ok(id), + Problem); } } -public record CreateTodoListCommand(string? Title) : IRequest; +public record CreateTodoListCommand(string? Title) : IRequest>; internal sealed class CreateTodoListCommandValidator : AbstractValidator { @@ -43,11 +49,11 @@ private Task BeUniqueTitle(string title, CancellationToken cancellationTok } } -internal sealed class CreateTodoListCommandHandler(ApplicationDbContext context) : IRequestHandler +internal sealed class CreateTodoListCommandHandler(ApplicationDbContext context) : IRequestHandler> { private readonly ApplicationDbContext _context = context; - public async Task Handle(CreateTodoListCommand request, CancellationToken cancellationToken) + public async Task> Handle(CreateTodoListCommand request, CancellationToken cancellationToken) { var todoList = new TodoList { Title = request.Title }; diff --git a/src/Application/Features/TodoLists/DeleteTodoList.cs b/src/Application/Features/TodoLists/DeleteTodoList.cs index 9d17202..06469c3 100644 --- a/src/Application/Features/TodoLists/DeleteTodoList.cs +++ b/src/Application/Features/TodoLists/DeleteTodoList.cs @@ -1,11 +1,10 @@ -using MediatR; +using ErrorOr; + +using MediatR; using Microsoft.AspNetCore.Mvc; -using Microsoft.EntityFrameworkCore; using VerticalSliceArchitecture.Application.Common; -using VerticalSliceArchitecture.Application.Common.Exceptions; -using VerticalSliceArchitecture.Application.Domain.Todos; using VerticalSliceArchitecture.Application.Infrastructure.Persistence; namespace VerticalSliceArchitecture.Application.Features.TodoLists; @@ -15,26 +14,34 @@ public class DeleteTodoListController : ApiControllerBase [HttpDelete("/api/todo-lists/{id}")] public async Task Delete(int id) { - await Mediator.Send(new DeleteTodoListCommand(id)); + var result = await Mediator.Send(new DeleteTodoListCommand(id)); - return NoContent(); + return result.Match( + _ => NoContent(), + Problem); } } -public record DeleteTodoListCommand(int Id) : IRequest; +public record DeleteTodoListCommand(int Id) : IRequest>; -internal sealed class DeleteTodoListCommandHandler(ApplicationDbContext context) : IRequestHandler +internal sealed class DeleteTodoListCommandHandler(ApplicationDbContext context) : IRequestHandler> { private readonly ApplicationDbContext _context = context; - public async Task Handle(DeleteTodoListCommand request, CancellationToken cancellationToken) + public async Task> Handle(DeleteTodoListCommand request, CancellationToken cancellationToken) { var todoList = await _context.TodoLists - .Where(l => l.Id == request.Id) - .SingleOrDefaultAsync(cancellationToken) ?? throw new NotFoundException(nameof(TodoList), request.Id); + .FindAsync([request.Id], cancellationToken); + + if (todoList is null) + { + return Error.NotFound(description: "TodoList not found."); + } _context.TodoLists.Remove(todoList); await _context.SaveChangesAsync(cancellationToken); + + return Result.Success; } } \ No newline at end of file From ee9ed55540a2fe0ecfc3ae47dc9cc83c0cd6fb51 Mon Sep 17 00:00:00 2001 From: Nadir Badnjevic Date: Thu, 26 Dec 2024 20:54:27 +0100 Subject: [PATCH 2/6] refactor TodoList features to use ErrorOr --- .vscode/settings.json | 51 +++++++++++-------- requests/TodoLists/CreateTodoList.http | 2 +- requests/TodoLists/ExportTodoLists.http | 5 ++ requests/TodoLists/UpdateTodoItem.http | 40 +++++++++++++++ ...tionBehaviour.cs => ValidationBehavior.cs} | 0 .../Features/TodoLists/ExportTodos.cs | 18 ++++--- .../Features/TodoLists/GetTodos.cs | 24 +++++---- .../Features/TodoLists/UpdateTodoList.cs | 35 ++++++++----- 8 files changed, 125 insertions(+), 50 deletions(-) create mode 100644 requests/TodoLists/ExportTodoLists.http create mode 100644 requests/TodoLists/UpdateTodoItem.http rename src/Application/Common/Behaviours/{ValidationBehaviour.cs => ValidationBehavior.cs} (100%) diff --git a/.vscode/settings.json b/.vscode/settings.json index a179839..42be871 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,23 +1,32 @@ { - "workbench.colorCustomizations": { - "activityBar.activeBackground": "#3399ff", - "activityBar.activeBorder": "#bf0060", - "activityBar.background": "#3399ff", - "activityBar.foreground": "#15202b", - "activityBar.inactiveForeground": "#15202b99", - "activityBarBadge.background": "#bf0060", - "activityBarBadge.foreground": "#e7e7e7", - "sash.hoverBorder": "#3399ff", - "statusBar.background": "#007fff", - "statusBar.foreground": "#e7e7e7", - "statusBarItem.hoverBackground": "#3399ff", - "statusBarItem.remoteBackground": "#007fff", - "statusBarItem.remoteForeground": "#e7e7e7", - "titleBar.activeBackground": "#007fff", - "titleBar.activeForeground": "#e7e7e7", - "titleBar.inactiveBackground": "#007fff99", - "titleBar.inactiveForeground": "#e7e7e799", - "commandCenter.border": "#e7e7e799" + "workbench.colorCustomizations": { + "activityBar.activeBackground": "#65c89b", + "activityBar.activeBorder": "#bf0060", + "activityBar.background": "#65c89b", + "activityBar.foreground": "#15202b", + "activityBar.inactiveForeground": "#15202b99", + "activityBarBadge.background": "#945bc4", + "activityBarBadge.foreground": "#e7e7e7", + "sash.hoverBorder": "#65c89b", + "statusBar.background": "#42b883", + "statusBar.foreground": "#15202b", + "statusBarItem.hoverBackground": "#359268", + "statusBarItem.remoteBackground": "#42b883", + "statusBarItem.remoteForeground": "#15202b", + "titleBar.activeBackground": "#42b883", + "titleBar.activeForeground": "#15202b", + "titleBar.inactiveBackground": "#42b88399", + "titleBar.inactiveForeground": "#15202b99", + "commandCenter.border": "#15202b99" + }, + "peacock.color": "#42b883", + + "rest-client.environmentVariables": { + "$shared": { + "listId": "0" }, - "peacock.color": "#007fff" -} \ No newline at end of file + "dev": { + "host": "https://localhost:7098" + } + } +} diff --git a/requests/TodoLists/CreateTodoList.http b/requests/TodoLists/CreateTodoList.http index 5dc691c..8e02d33 100644 --- a/requests/TodoLists/CreateTodoList.http +++ b/requests/TodoLists/CreateTodoList.http @@ -21,5 +21,5 @@ POST {{host}}/api/todo-lists Content-Type: application/json { - "title": "List 1" + "title": "List 2" } \ No newline at end of file diff --git a/requests/TodoLists/ExportTodoLists.http b/requests/TodoLists/ExportTodoLists.http new file mode 100644 index 0000000..3ebb6ef --- /dev/null +++ b/requests/TodoLists/ExportTodoLists.http @@ -0,0 +1,5 @@ +@host = https://localhost:7098 +@todoListId = 2 + +### Export Todo lists +GET {{host}}/api/todo-lists/{{todoListId}} \ No newline at end of file diff --git a/requests/TodoLists/UpdateTodoItem.http b/requests/TodoLists/UpdateTodoItem.http new file mode 100644 index 0000000..5c65f33 --- /dev/null +++ b/requests/TodoLists/UpdateTodoItem.http @@ -0,0 +1,40 @@ +@host = https://localhost:7098 +@todoListId = 1 + +### Update TodoListId = 1 with title = "Updated TodoList 1" +PUT {{host}}/api/todo-lists/{{todoListId}} +Content-Type: application/json + +{ + "id": {{todoListId}}, + "title": "Updated Todo Item 1" +} + +### Update TodoListId = 1 with wrong Id +PUT {{host}}/api/todo-items/0 +Content-Type: application/json + +{ + "id": {{todoListId}}, + "title": "Updated Todo Item 1" +} + +### Update TodoListId = 1 with empty title +PUT {{host}}/api/todo-lists/{{todoListId}} +Content-Type: application/json + +{ + "id": {{todoListId}}, + "title": "" +} + +### Update TodoListId = 1 with title more than 200 characters +PUT {{host}}/api/todo-lists/{{todoListId}} +Content-Type: application/json + +{ + "id": {{todoListId}}, + "title": "Lorem ipsum dolor sit amet, consectetur adipiscing elit Lorem ipsum dolor sit amet, consectetur adipiscing elit Lorem ipsum dolor sit amet, consectetur adipiscing elit Lorem ipsum dolor sit amet, consectetur adipiscing elit Lorem ipsum dolor sit amet, consectetur adipiscing elit" +} + + diff --git a/src/Application/Common/Behaviours/ValidationBehaviour.cs b/src/Application/Common/Behaviours/ValidationBehavior.cs similarity index 100% rename from src/Application/Common/Behaviours/ValidationBehaviour.cs rename to src/Application/Common/Behaviours/ValidationBehavior.cs diff --git a/src/Application/Features/TodoLists/ExportTodos.cs b/src/Application/Features/TodoLists/ExportTodos.cs index 3e4be29..1d607db 100644 --- a/src/Application/Features/TodoLists/ExportTodos.cs +++ b/src/Application/Features/TodoLists/ExportTodos.cs @@ -1,4 +1,6 @@ -using MediatR; +using ErrorOr; + +using MediatR; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; @@ -13,24 +15,26 @@ namespace VerticalSliceArchitecture.Application.Features.TodoLists; public class ExportTodosController : ApiControllerBase { [HttpGet("/api/todo-lists/{id}")] - public async Task Get(int id) + public async Task Get(int id) { - var vm = await Mediator.Send(new ExportTodosQuery(id)); + var result = await Mediator.Send(new ExportTodosQuery(id)); - return File(vm.Content, vm.ContentType, vm.FileName); + return result.Match( + vm => File(vm.Content, vm.ContentType, vm.FileName), + Problem); } } -public record ExportTodosQuery(int ListId) : IRequest; +public record ExportTodosQuery(int ListId) : IRequest>; public record ExportTodosVm(string FileName, string ContentType, byte[] Content); -internal sealed class ExportTodosQueryHandler(ApplicationDbContext context, ICsvFileBuilder fileBuilder) : IRequestHandler +internal sealed class ExportTodosQueryHandler(ApplicationDbContext context, ICsvFileBuilder fileBuilder) : IRequestHandler> { private readonly ApplicationDbContext _context = context; private readonly ICsvFileBuilder _fileBuilder = fileBuilder; - public async Task Handle(ExportTodosQuery request, CancellationToken cancellationToken) + public async Task> Handle(ExportTodosQuery request, CancellationToken cancellationToken) { var records = await _context.TodoItems .Where(t => t.ListId == request.ListId) diff --git a/src/Application/Features/TodoLists/GetTodos.cs b/src/Application/Features/TodoLists/GetTodos.cs index a30b74f..bfea218 100644 --- a/src/Application/Features/TodoLists/GetTodos.cs +++ b/src/Application/Features/TodoLists/GetTodos.cs @@ -1,4 +1,6 @@ -using MediatR; +using ErrorOr; + +using MediatR; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; @@ -12,19 +14,23 @@ namespace VerticalSliceArchitecture.Application.Features.TodoLists; public class GetTodosController : ApiControllerBase { [HttpGet("/api/todo-lists")] - public async Task> Get() + public async Task Get() { - return await Mediator.Send(new GetTodosQuery()); + var result = await Mediator.Send(new GetTodosQuery()); + + return result.Match( + Ok, + Problem); } } -public record GetTodosQuery : IRequest; +public record GetTodosQuery : IRequest>; public class TodosVm { - public IList PriorityLevels { get; set; } = new List(); + public IList PriorityLevels { get; set; } = []; - public IList Lists { get; set; } = new List(); + public IList Lists { get; set; } = []; } public record PriorityLevelDto(int Value, string? Name); @@ -32,18 +38,18 @@ public record PriorityLevelDto(int Value, string? Name); public record TodoListDto(int Id, string? Title, string? Colour, IList Items) { public TodoListDto() - : this(default, null, null, new List()) + : this(default, null, null, []) { } } public record TodoItemDto(int Id, int ListId, string? Title, bool Done, int Priority, string? Note); -internal sealed class GetTodosQueryHandler(ApplicationDbContext context) : IRequestHandler +internal sealed class GetTodosQueryHandler(ApplicationDbContext context) : IRequestHandler> { private readonly ApplicationDbContext _context = context; - public async Task Handle(GetTodosQuery request, CancellationToken cancellationToken) + public async Task> Handle(GetTodosQuery request, CancellationToken cancellationToken) { return new TodosVm { diff --git a/src/Application/Features/TodoLists/UpdateTodoList.cs b/src/Application/Features/TodoLists/UpdateTodoList.cs index 1d89092..d8264aa 100644 --- a/src/Application/Features/TodoLists/UpdateTodoList.cs +++ b/src/Application/Features/TodoLists/UpdateTodoList.cs @@ -1,13 +1,14 @@ -using FluentValidation; +using ErrorOr; + +using FluentValidation; using MediatR; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; using VerticalSliceArchitecture.Application.Common; -using VerticalSliceArchitecture.Application.Common.Exceptions; -using VerticalSliceArchitecture.Application.Domain.Todos; using VerticalSliceArchitecture.Application.Infrastructure.Persistence; namespace VerticalSliceArchitecture.Application.Features.TodoLists; @@ -15,20 +16,24 @@ namespace VerticalSliceArchitecture.Application.Features.TodoLists; public class UpdateTodoListController : ApiControllerBase { [HttpPut("/api/todo-lists/{id}")] - public async Task Update(int id, UpdateTodoListCommand command) + public async Task Update(int id, UpdateTodoListCommand command) { if (id != command.Id) { - return BadRequest(); + return Problem( + statusCode: StatusCodes.Status400BadRequest, + detail: "Not matching ids"); } - await Mediator.Send(command); + var result = await Mediator.Send(command); - return NoContent(); + return result.Match( + _ => NoContent(), + Problem); } } -public class UpdateTodoListCommand : IRequest +public class UpdateTodoListCommand : IRequest> { public int Id { get; set; } @@ -57,7 +62,7 @@ public Task BeUniqueTitle(UpdateTodoListCommand model, string title, Cance } } -internal sealed class UpdateTodoListCommandHandler : IRequestHandler +internal sealed class UpdateTodoListCommandHandler : IRequestHandler> { private readonly ApplicationDbContext _context; @@ -66,14 +71,20 @@ public UpdateTodoListCommandHandler(ApplicationDbContext context) _context = context; } - public async Task Handle(UpdateTodoListCommand request, CancellationToken cancellationToken) + public async Task> Handle(UpdateTodoListCommand request, CancellationToken cancellationToken) { var entity = await _context.TodoLists - .FindAsync(new object[] { request.Id }, cancellationToken) - .ConfigureAwait(false) ?? throw new NotFoundException(nameof(TodoList), request.Id); + .FindAsync([request.Id], cancellationToken); + + if (entity is null) + { + return Error.NotFound(description: "TodoList not found."); + } entity.Title = request.Title; await _context.SaveChangesAsync(cancellationToken); + + return Result.Success; } } \ No newline at end of file From c408efc1ef6ab6e8bfc5bcb93787282c3f1f61de Mon Sep 17 00:00:00 2001 From: Nadir Badnjevic Date: Thu, 26 Dec 2024 20:55:34 +0100 Subject: [PATCH 3/6] refactor: remove exceptions and tests --- Directory.Packages.props | 1 + .../Filters/ApiExceptionFilterAttribute.cs | 132 ------------------ .../Exceptions/ForbiddenAccessException.cs | 19 --- .../Common/Exceptions/NotFoundException.cs | 24 ---- .../Common/Exceptions/ValidationException.cs | 22 --- .../Application.UnitTests.csproj | 1 + .../Behaviours/ValidationBehaviorTests.cs | 84 +++++++++++ .../Exceptions/ValidationExceptionTests.cs | 62 -------- tests/Application.UnitTests/GlobalUsings.cs | 4 + 9 files changed, 90 insertions(+), 259 deletions(-) delete mode 100644 src/Api/Filters/ApiExceptionFilterAttribute.cs delete mode 100644 src/Application/Common/Exceptions/ForbiddenAccessException.cs delete mode 100644 src/Application/Common/Exceptions/NotFoundException.cs delete mode 100644 src/Application/Common/Exceptions/ValidationException.cs create mode 100644 tests/Application.UnitTests/Common/Behaviours/ValidationBehaviorTests.cs delete mode 100644 tests/Application.UnitTests/Common/Exceptions/ValidationExceptionTests.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index f2285ff..c42aa0e 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -28,5 +28,6 @@ + diff --git a/src/Api/Filters/ApiExceptionFilterAttribute.cs b/src/Api/Filters/ApiExceptionFilterAttribute.cs deleted file mode 100644 index a5168fe..0000000 --- a/src/Api/Filters/ApiExceptionFilterAttribute.cs +++ /dev/null @@ -1,132 +0,0 @@ -using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Mvc.Filters; - -using VerticalSliceArchitecture.Application.Common.Exceptions; - -namespace VerticalSliceArchitecture.Api.Filters; - -public class ApiExceptionFilterAttribute : ExceptionFilterAttribute -{ - private readonly IDictionary> _exceptionHandlers; - - public ApiExceptionFilterAttribute() - { - // Register known exception types and handlers. - _exceptionHandlers = new Dictionary> - { - { typeof(ValidationException), HandleValidationException }, - { typeof(NotFoundException), HandleNotFoundException }, - { typeof(UnauthorizedAccessException), HandleUnauthorizedAccessException }, - { typeof(ForbiddenAccessException), HandleForbiddenAccessException }, - }; - } - - public override void OnException(ExceptionContext context) - { - HandleException(context); - - base.OnException(context); - } - - private void HandleException(ExceptionContext context) - { - Type type = context.Exception.GetType(); - if (_exceptionHandlers.ContainsKey(type)) - { - _exceptionHandlers[type].Invoke(context); - return; - } - - if (!context.ModelState.IsValid) - { - HandleInvalidModelStateException(context); - return; - } - - HandleUnknownException(context); - } - - private void HandleValidationException(ExceptionContext context) - { - ValidationException? exception = context.Exception as ValidationException; - - ValidationProblemDetails details = new(exception!.Errors) - { - Type = "https://tools.ietf.org/html/rfc7231#section-6.5.1", - }; - - context.Result = new BadRequestObjectResult(details); - - context.ExceptionHandled = true; - } - - private void HandleInvalidModelStateException(ExceptionContext context) - { - ValidationProblemDetails details = new(context.ModelState) - { - Type = "https://tools.ietf.org/html/rfc7231#section-6.5.1", - }; - - context.Result = new BadRequestObjectResult(details); - - context.ExceptionHandled = true; - } - - private void HandleNotFoundException(ExceptionContext context) - { - NotFoundException? exception = context.Exception as NotFoundException; - - ProblemDetails details = new() - { - Type = "https://tools.ietf.org/html/rfc7231#section-6.5.4", - Title = "The specified resource was not found.", - Detail = exception!.Message, - }; - - context.Result = new NotFoundObjectResult(details); - - context.ExceptionHandled = true; - } - - private void HandleUnauthorizedAccessException(ExceptionContext context) - { - ProblemDetails details = new() - { - Status = StatusCodes.Status401Unauthorized, - Title = "Unauthorized", - Type = "https://tools.ietf.org/html/rfc7235#section-3.1", - }; - - context.Result = new ObjectResult(details) { StatusCode = StatusCodes.Status401Unauthorized }; - - context.ExceptionHandled = true; - } - - private void HandleForbiddenAccessException(ExceptionContext context) - { - ProblemDetails details = new() - { - Status = StatusCodes.Status403Forbidden, - Title = "Forbidden", - Type = "https://tools.ietf.org/html/rfc7231#section-6.5.3", - }; - - context.Result = new ObjectResult(details) { StatusCode = StatusCodes.Status403Forbidden }; - - context.ExceptionHandled = true; - } - - private void HandleUnknownException(ExceptionContext context) - { - ProblemDetails details = new() - { - Status = StatusCodes.Status500InternalServerError, - Title = "An error occurred while processing your request.", - Type = "https://tools.ietf.org/html/rfc7231#section-6.6.1", - }; - - context.Result = new ObjectResult(details) { StatusCode = StatusCodes.Status500InternalServerError }; - - context.ExceptionHandled = true; - } -} \ No newline at end of file diff --git a/src/Application/Common/Exceptions/ForbiddenAccessException.cs b/src/Application/Common/Exceptions/ForbiddenAccessException.cs deleted file mode 100644 index ca35f2a..0000000 --- a/src/Application/Common/Exceptions/ForbiddenAccessException.cs +++ /dev/null @@ -1,19 +0,0 @@ -using System.Runtime.Serialization; - -namespace VerticalSliceArchitecture.Application.Common.Exceptions; - -public class ForbiddenAccessException : Exception -{ - public ForbiddenAccessException() - : base() { } - - public ForbiddenAccessException(string? message) - : base(message) - { - } - - public ForbiddenAccessException(string? message, Exception? innerException) - : base(message, innerException) - { - } -} \ No newline at end of file diff --git a/src/Application/Common/Exceptions/NotFoundException.cs b/src/Application/Common/Exceptions/NotFoundException.cs deleted file mode 100644 index e5a5344..0000000 --- a/src/Application/Common/Exceptions/NotFoundException.cs +++ /dev/null @@ -1,24 +0,0 @@ -namespace VerticalSliceArchitecture.Application.Common.Exceptions; - -public class NotFoundException : Exception -{ - public NotFoundException() - : base() - { - } - - public NotFoundException(string message) - : base(message) - { - } - - public NotFoundException(string message, Exception innerException) - : base(message, innerException) - { - } - - public NotFoundException(string name, object key) - : base($"Entity '{name}' ({key}) was not found.") - { - } -} \ No newline at end of file diff --git a/src/Application/Common/Exceptions/ValidationException.cs b/src/Application/Common/Exceptions/ValidationException.cs deleted file mode 100644 index f80dc5b..0000000 --- a/src/Application/Common/Exceptions/ValidationException.cs +++ /dev/null @@ -1,22 +0,0 @@ -using FluentValidation.Results; - -namespace VerticalSliceArchitecture.Application.Common.Exceptions; - -public class ValidationException : Exception -{ - public ValidationException() - : base("One or more validation failures have occurred.") - { - Errors = new Dictionary(); - } - - public ValidationException(IEnumerable failures) - : this() - { - Errors = failures - .GroupBy(e => e.PropertyName, e => e.ErrorMessage) - .ToDictionary(failureGroup => failureGroup.Key, failureGroup => failureGroup.ToArray()); - } - - public IDictionary Errors { get; } -} \ No newline at end of file diff --git a/tests/Application.UnitTests/Application.UnitTests.csproj b/tests/Application.UnitTests/Application.UnitTests.csproj index c7c97f0..af868f9 100644 --- a/tests/Application.UnitTests/Application.UnitTests.csproj +++ b/tests/Application.UnitTests/Application.UnitTests.csproj @@ -8,6 +8,7 @@ + runtime; build; native; contentfiles; analyzers; buildtransitive diff --git a/tests/Application.UnitTests/Common/Behaviours/ValidationBehaviorTests.cs b/tests/Application.UnitTests/Common/Behaviours/ValidationBehaviorTests.cs new file mode 100644 index 0000000..67e177f --- /dev/null +++ b/tests/Application.UnitTests/Common/Behaviours/ValidationBehaviorTests.cs @@ -0,0 +1,84 @@ +using FluentValidation; +using FluentValidation.Results; + +using MediatR; + +using VerticalSliceArchitecture.Application.Common.Behaviours; +using VerticalSliceArchitecture.Application.Domain.Todos; +using VerticalSliceArchitecture.Application.Features.TodoLists; + +namespace VerticalSliceArchitecture.Application.UnitTests.Common.Behaviours; + +public class ValidationBehaviorTests +{ + private readonly ValidationBehavior> _validationBehavior; + private readonly IValidator _mockValidator; + private readonly RequestHandlerDelegate> _mockNextBehavior; + + public ValidationBehaviorTests() + { + _mockNextBehavior = Substitute.For>>(); + _mockValidator = Substitute.For>(); + + _validationBehavior = new(_mockValidator); + } + + [Fact] + public async Task InvokeValidationBehavior_WhenValidatorResultIsValid_ShouldInvokeNextBehavior() + { + // Arrange + var createTodoListCommand = new CreateTodoListCommand("Title"); + var todoList = new TodoList { Title = createTodoListCommand.Title }; + + _mockValidator + .ValidateAsync(createTodoListCommand, Arg.Any()) + .Returns(new ValidationResult()); + + _mockNextBehavior.Invoke().Returns(todoList.Id); + + // Act + var result = await _validationBehavior.Handle(createTodoListCommand, _mockNextBehavior, default); + + // Assert + result.IsError.Should().BeFalse(); + result.Value.Should().Be(todoList.Id); + } + + [Fact] + public async Task InvokeValidationBehavior_WhenValidatorResultIsNotValid_ShouldReturnListOfErrors() + { + // Arrange + var createTodoListCommand = new CreateTodoListCommand("Title"); + List validationFailures = [new(propertyName: "foo", errorMessage: "bad foo")]; + + _mockValidator + .ValidateAsync(createTodoListCommand, Arg.Any()) + .Returns(new ValidationResult(validationFailures)); + + // Act + var result = await _validationBehavior.Handle(createTodoListCommand, _mockNextBehavior, default); + + // Assert + result.IsError.Should().BeTrue(); + result.FirstError.Code.Should().Be("foo"); + result.FirstError.Description.Should().Be("bad foo"); + } + + [Fact] + public async Task InvokeValidationBehavior_WhenNoValidator_ShouldInvokeNextBehavior() + { + // Arrange + var createTodoListCommand = new CreateTodoListCommand("Title"); + var validationBehavior = new ValidationBehavior>(); + + var todoList = new TodoList { Title = createTodoListCommand.Title }; + _mockNextBehavior.Invoke().Returns(todoList.Id); + + // Act + var result = await validationBehavior.Handle(createTodoListCommand, _mockNextBehavior, default); + + // Assert + result.IsError.Should().BeFalse(); + result.Value.Should().Be(todoList.Id); + } +} \ No newline at end of file diff --git a/tests/Application.UnitTests/Common/Exceptions/ValidationExceptionTests.cs b/tests/Application.UnitTests/Common/Exceptions/ValidationExceptionTests.cs deleted file mode 100644 index d401eca..0000000 --- a/tests/Application.UnitTests/Common/Exceptions/ValidationExceptionTests.cs +++ /dev/null @@ -1,62 +0,0 @@ -using FluentValidation.Results; - -using VerticalSliceArchitecture.Application.Common.Exceptions; - -namespace VerticalSliceArchitecture.Application.UnitTests.Common.Exceptions; - -public class ValidationExceptionTests -{ - [Fact] - public void DefaultConstructorCreatesAnEmptyErrorDictionary() - { - var actual = new ValidationException().Errors; - - actual.Keys.Should().BeEquivalentTo(Array.Empty()); - } - - [Fact] - public void SingleValidationFailureCreatesASingleElementErrorDictionary() - { - var failures = new List - { - new("Age", "must be over 18"), - }; - - var actual = new ValidationException(failures).Errors; - - actual.Keys.Should().BeEquivalentTo(new string[] { "Age" }); - actual["Age"].Should().BeEquivalentTo(new string[] { "must be over 18" }); - } - - [Fact] - public void MulitpleValidationFailureForMultiplePropertiesCreatesAMultipleElementErrorDictionaryEachWithMultipleValues() - { - var failures = new List - { - new("Age", "must be 18 or older"), - new("Age", "must be 25 or younger"), - new("Password", "must contain at least 8 characters"), - new("Password", "must contain a digit"), - new("Password", "must contain upper case letter"), - new("Password", "must contain lower case letter"), - }; - - var actual = new ValidationException(failures).Errors; - - actual.Keys.Should().BeEquivalentTo(new string[] { "Password", "Age" }); - - actual["Age"].Should().BeEquivalentTo(new string[] - { - "must be 25 or younger", - "must be 18 or older", - }); - - actual["Password"].Should().BeEquivalentTo(new string[] - { - "must contain lower case letter", - "must contain upper case letter", - "must contain at least 8 characters", - "must contain a digit", - }); - } -} \ No newline at end of file diff --git a/tests/Application.UnitTests/GlobalUsings.cs b/tests/Application.UnitTests/GlobalUsings.cs index 168410c..fae4ea1 100644 --- a/tests/Application.UnitTests/GlobalUsings.cs +++ b/tests/Application.UnitTests/GlobalUsings.cs @@ -1,3 +1,7 @@ +global using ErrorOr; + global using FluentAssertions; +global using NSubstitute; + global using Xunit; \ No newline at end of file From 5a2a2c2cfee28f90a26dc88ad262ecc779f073ba Mon Sep 17 00:00:00 2001 From: Nadir Badnjevic Date: Fri, 27 Dec 2024 08:20:32 +0100 Subject: [PATCH 4/6] rename ValidationBehaviour --- .../{ValidationBehavior.cs => ValidationBehaviour.cs} | 2 +- src/Application/ConfigureServices.cs | 2 +- .../Common/Behaviours/ValidationBehaviorTests.cs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) rename src/Application/Common/Behaviours/{ValidationBehavior.cs => ValidationBehaviour.cs} (88%) diff --git a/src/Application/Common/Behaviours/ValidationBehavior.cs b/src/Application/Common/Behaviours/ValidationBehaviour.cs similarity index 88% rename from src/Application/Common/Behaviours/ValidationBehavior.cs rename to src/Application/Common/Behaviours/ValidationBehaviour.cs index 26e4165..e471866 100644 --- a/src/Application/Common/Behaviours/ValidationBehavior.cs +++ b/src/Application/Common/Behaviours/ValidationBehaviour.cs @@ -6,7 +6,7 @@ namespace VerticalSliceArchitecture.Application.Common.Behaviours; -public class ValidationBehavior(IValidator? validator = null) +public class ValidationBehaviour(IValidator? validator = null) : IPipelineBehavior where TRequest : IRequest where TResponse : IErrorOr diff --git a/src/Application/ConfigureServices.cs b/src/Application/ConfigureServices.cs index 1fbc9d1..7ecceb7 100644 --- a/src/Application/ConfigureServices.cs +++ b/src/Application/ConfigureServices.cs @@ -22,7 +22,7 @@ public static IServiceCollection AddApplication(this IServiceCollection services options.AddOpenBehavior(typeof(AuthorizationBehaviour<,>)); options.AddOpenBehavior(typeof(PerformanceBehaviour<,>)); - options.AddOpenBehavior(typeof(ValidationBehavior<,>)); + options.AddOpenBehavior(typeof(ValidationBehaviour<,>)); }); services.AddValidatorsFromAssembly(typeof(DependencyInjection).Assembly, includeInternalTypes: true); diff --git a/tests/Application.UnitTests/Common/Behaviours/ValidationBehaviorTests.cs b/tests/Application.UnitTests/Common/Behaviours/ValidationBehaviorTests.cs index 67e177f..c5a5386 100644 --- a/tests/Application.UnitTests/Common/Behaviours/ValidationBehaviorTests.cs +++ b/tests/Application.UnitTests/Common/Behaviours/ValidationBehaviorTests.cs @@ -11,7 +11,7 @@ namespace VerticalSliceArchitecture.Application.UnitTests.Common.Behaviours; public class ValidationBehaviorTests { - private readonly ValidationBehavior> _validationBehavior; + private readonly ValidationBehaviour> _validationBehavior; private readonly IValidator _mockValidator; private readonly RequestHandlerDelegate> _mockNextBehavior; @@ -69,7 +69,7 @@ public async Task InvokeValidationBehavior_WhenNoValidator_ShouldInvokeNextBehav { // Arrange var createTodoListCommand = new CreateTodoListCommand("Title"); - var validationBehavior = new ValidationBehavior>(); + var validationBehavior = new ValidationBehaviour>(); var todoList = new TodoList { Title = createTodoListCommand.Title }; _mockNextBehavior.Invoke().Returns(todoList.Id); From 4aad452999abdaf247f3f96bbb52a2261202f288 Mon Sep 17 00:00:00 2001 From: Nadir Badnjevic Date: Fri, 27 Dec 2024 08:30:39 +0100 Subject: [PATCH 5/6] refactor: remove exceptions --- .../Exceptions/UnsupportedColourException.cs | 20 ----------------- src/Application/Domain/ValueObjects/Colour.cs | 16 ++++++-------- .../Domain/ValueObjects/ColourErrors.cs | 11 ++++++++++ .../ValueObjects/ColourTests.cs | 22 ++++++++----------- 4 files changed, 27 insertions(+), 42 deletions(-) delete mode 100644 src/Application/Common/Exceptions/UnsupportedColourException.cs create mode 100644 src/Application/Domain/ValueObjects/ColourErrors.cs diff --git a/src/Application/Common/Exceptions/UnsupportedColourException.cs b/src/Application/Common/Exceptions/UnsupportedColourException.cs deleted file mode 100644 index 845c844..0000000 --- a/src/Application/Common/Exceptions/UnsupportedColourException.cs +++ /dev/null @@ -1,20 +0,0 @@ -using System.Runtime.Serialization; - -namespace VerticalSliceArchitecture.Application.Common.Exceptions; - -public class UnsupportedColourException : Exception -{ - public UnsupportedColourException() - { - } - - public UnsupportedColourException(string code) - : base($"Colour '{code}' is unsupported.") - { - } - - public UnsupportedColourException(string? message, Exception? innerException) - : base(message, innerException) - { - } -} \ No newline at end of file diff --git a/src/Application/Domain/ValueObjects/Colour.cs b/src/Application/Domain/ValueObjects/Colour.cs index 6cbd804..b0d47f4 100644 --- a/src/Application/Domain/ValueObjects/Colour.cs +++ b/src/Application/Domain/ValueObjects/Colour.cs @@ -1,5 +1,8 @@ -using VerticalSliceArchitecture.Application.Common; -using VerticalSliceArchitecture.Application.Common.Exceptions; +using System.Reflection.Metadata.Ecma335; + +using ErrorOr; + +using VerticalSliceArchitecture.Application.Common; namespace VerticalSliceArchitecture.Application.Domain.ValueObjects; @@ -18,11 +21,11 @@ private Colour(string code) Code = code; } - public static Colour From(string code) + public static ErrorOr From(string code) { var colour = new Colour { Code = code }; - return !SupportedColours.Contains(colour) ? throw new UnsupportedColourException(code) : colour; + return !SupportedColours.Contains(colour) ? ColourErrors.UnsupportedColour(code) : colour; } public static Colour White => new("#FFFFFF"); @@ -48,11 +51,6 @@ public static implicit operator string(Colour colour) return colour.ToString(); } - public static explicit operator Colour(string code) - { - return From(code); - } - public override string ToString() { return Code; diff --git a/src/Application/Domain/ValueObjects/ColourErrors.cs b/src/Application/Domain/ValueObjects/ColourErrors.cs new file mode 100644 index 0000000..8811134 --- /dev/null +++ b/src/Application/Domain/ValueObjects/ColourErrors.cs @@ -0,0 +1,11 @@ +using ErrorOr; + +namespace VerticalSliceArchitecture.Application.Domain.ValueObjects; + +public static class ColourErrors +{ + public static Error UnsupportedColour(string code) => + Error.Validation( + code: "ColourErrors.UnsupportedColour", + description: $"The colour code '{code}' is not supported."); +} \ No newline at end of file diff --git a/tests/Application.UnitTests/ValueObjects/ColourTests.cs b/tests/Application.UnitTests/ValueObjects/ColourTests.cs index 9bc87c7..207305a 100644 --- a/tests/Application.UnitTests/ValueObjects/ColourTests.cs +++ b/tests/Application.UnitTests/ValueObjects/ColourTests.cs @@ -1,5 +1,4 @@ -using VerticalSliceArchitecture.Application.Common.Exceptions; -using VerticalSliceArchitecture.Application.Domain.ValueObjects; +using VerticalSliceArchitecture.Application.Domain.ValueObjects; namespace VerticalSliceArchitecture.Application.UnitTests.ValueObjects; @@ -12,7 +11,8 @@ public void ShouldReturnCorrectColourCode() var colour = Colour.From(code); - colour.Code.Should().Be(code); + colour.IsError.Should().BeFalse(); + colour.Value.Code.Should().Be(code); } [Fact] @@ -32,17 +32,13 @@ public void ShouldPerformImplicitConversionToColourCodeString() } [Fact] - public void ShouldPerformExplicitConversionGivenSupportedColourCode() + public void ShouldReturnErrorGivenNotSupportedColourCode() { - var colour = (Colour)"#FFFFFF"; + // Arrange/Act + var colour = Colour.From("##FF33CC"); - colour.Should().Be(Colour.White); - } - - [Fact] - public void ShouldThrowUnsupportedColourExceptionGivenNotSupportedColourCode() - { - FluentActions.Invoking(() => Colour.From("##FF33CC")) - .Should().Throw(); + // Assert + colour.IsError.Should().BeTrue(); + colour.FirstError.Code.Should().Be("ColourErrors.UnsupportedColour"); } } \ No newline at end of file From fd4ccbf80068d226f7f37fd4f19c24610570df4c Mon Sep 17 00:00:00 2001 From: Nadir Badnjevic Date: Mon, 20 Jan 2025 17:05:16 +0100 Subject: [PATCH 6/6] fix: code analysis settings --- Directory.Build.props | 3 +-- README.md | 17 +++++++++++------ src/Api/ErrorController.cs | 2 ++ .../Services/DomainEventService.cs | 2 +- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index 8636308..06e3143 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -7,9 +7,8 @@ VerticalSliceArchitecture.$(MSBuildProjectName) $(AssemblyName) - 8-minimal + 8-minimum true - false true diff --git a/README.md b/README.md index 38e9741..615f0ba 100644 --- a/README.md +++ b/README.md @@ -148,18 +148,23 @@ dotnet ef database update --project src/Application --startup-project src/Api Developers should follow Microsoft's [C# Coding Conventions](https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/inside-a-program/coding-conventions). -To enforce consistent coding styles and settings in the codebase, we use an EditorConfig file (**.editorconfig**) prepopulated with the default [.NET code style, formatting, and naming conventions](https://docs.microsoft.com/en-us/visualstudio/ide/editorconfig-code-style-settings-reference?view=vs-2019). +To enforce consistent coding styles and settings in the codebase, we use an EditorConfig file (**.editorconfig**) prepopulated with the default [.NET code style, formatting, and naming conventions](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/code-style-rule-options). + +For [code analysis](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/overview?tabs=net-9) +we use the built in analyzers. **IMPORTANT NOTES:** - EditorConfig settings take precedence over global IDE text editor settings. - New lines of code are formatted according to the EditorConfig settings -- The formatting of existing code is not changed unless you run one of the following commands (Visual Studio): - - Code Cleanup (**Ctrl+K, Ctrl+E**) which applies any white space setting, indent style, and other code style settings. - - Format Document (**Ctrl+K, Ctrl+D**) +- **The formatting of existing code is not changed unless you run**: + - `dotnet format` from the command line -For [code analysis](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/overview?tabs=net-8) -we use the built in analyzers. +There are a few arguments we can supply to the dotnet format command to control its usage. A useful one is the `--verify-no-changes argument`. This argument is useful when we want to understand when code breaks standards, but not automatically clean it up. + +```shell +dotnet format --verify-no-changes +``` Both code formating and analysis can be performed from the cli by running: diff --git a/src/Api/ErrorController.cs b/src/Api/ErrorController.cs index 802f32a..70f2b5a 100644 --- a/src/Api/ErrorController.cs +++ b/src/Api/ErrorController.cs @@ -1,6 +1,8 @@ using Microsoft.AspNetCore.Diagnostics; using Microsoft.AspNetCore.Mvc; +namespace VerticalSliceArchitecture.Api; + public class ErrorController : ControllerBase { [Route("/error-development")] diff --git a/src/Application/Infrastructure/Services/DomainEventService.cs b/src/Application/Infrastructure/Services/DomainEventService.cs index a96535c..e36300a 100644 --- a/src/Application/Infrastructure/Services/DomainEventService.cs +++ b/src/Application/Infrastructure/Services/DomainEventService.cs @@ -25,7 +25,7 @@ public Task Publish(DomainEvent domainEvent) return _mediator.Publish(GetNotificationCorrespondingToDomainEvent(domainEvent)); } - private INotification GetNotificationCorrespondingToDomainEvent(DomainEvent domainEvent) + private static INotification GetNotificationCorrespondingToDomainEvent(DomainEvent domainEvent) { return (INotification)Activator.CreateInstance( typeof(DomainEventNotification<>).MakeGenericType(domainEvent.GetType()), domainEvent)!;