-
Notifications
You must be signed in to change notification settings - Fork 0
Order service: extract Order bounded context from monolith (decoupled, JWT, EF/Npgsql) #58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,29 +1,148 @@ | ||
| using Microsoft.AspNetCore.Authorization; | ||
| using Microsoft.AspNetCore.Mvc; | ||
| using Microsoft.EntityFrameworkCore; | ||
| using Order.API.Mapping; | ||
| using Order.API.ViewModels; | ||
| using Order.Infrastructure.Data; | ||
|
|
||
| namespace Order.API.Controllers; | ||
|
|
||
| // Resource controller mounted at the service ROOT. The API gateway strips the | ||
| // "/api/orders" prefix, so external "GET /api/orders" -> "GET /" here and | ||
| // "GET /api/orders/{id}" -> "GET /{id}". Absolute routes ("/...") are used so the | ||
| // service-internal paths match the gateway-stripped paths exactly. | ||
| [ApiController] | ||
| [Route("api/[controller]")] | ||
| [Authorize] | ||
| [Produces("application/json")] | ||
| [ProducesResponseType(StatusCodes.Status401Unauthorized)] | ||
| public class OrderController : ControllerBase | ||
| { | ||
| private readonly OrderDbContext _db; | ||
| private readonly ILogger<OrderController> _logger; | ||
|
|
||
| public OrderController(ILogger<OrderController> logger) | ||
| public OrderController(OrderDbContext db, ILogger<OrderController> logger) | ||
| { | ||
| _db = db; | ||
| _logger = logger; | ||
| } | ||
|
|
||
| [HttpGet] | ||
| public IActionResult GetAll() | ||
| [HttpGet("/")] | ||
| [ProducesResponseType(typeof(IEnumerable<OrderVM>), StatusCodes.Status200OK)] | ||
| public async Task<IActionResult> GetAll() | ||
| { | ||
| // TODO: Implement — migrate logic from monolith's OrderController | ||
| return Ok(new { service = "Order", status = "scaffold" }); | ||
| var orders = await _db.Orders | ||
| .AsNoTracking() | ||
| .Include(o => o.OrderDetails) | ||
| .ToListAsync(); | ||
|
|
||
| return Ok(orders.Select(o => o.ToViewModel())); | ||
| } | ||
|
|
||
| [HttpGet("/{id:int}")] | ||
| [ProducesResponseType(typeof(OrderVM), StatusCodes.Status200OK)] | ||
| [ProducesResponseType(StatusCodes.Status404NotFound)] | ||
| public async Task<IActionResult> GetById(int id) | ||
| { | ||
| var order = await _db.Orders | ||
| .AsNoTracking() | ||
| .Include(o => o.OrderDetails) | ||
| .FirstOrDefaultAsync(o => o.Id == id); | ||
|
|
||
| if (order is null) | ||
| return NotFound(id); | ||
|
|
||
| return Ok(order.ToViewModel()); | ||
| } | ||
|
|
||
| [HttpPost("/")] | ||
| [ProducesResponseType(typeof(OrderVM), StatusCodes.Status201Created)] | ||
| [ProducesResponseType(StatusCodes.Status400BadRequest)] | ||
| public async Task<IActionResult> Create([FromBody] OrderVM model) | ||
| { | ||
| if (!ModelState.IsValid) | ||
| return BadRequest(ModelState); | ||
|
|
||
| var order = model.ToEntity(); | ||
| foreach (var line in model.OrderDetails) | ||
| order.OrderDetails.Add(line.ToEntity()); | ||
|
|
||
| var now = DateTime.UtcNow; | ||
| var userName = User.Identity?.Name; | ||
| Stamp(order, now, userName, isNew: true); | ||
| foreach (var detail in order.OrderDetails) | ||
| Stamp(detail, now, userName, isNew: true); | ||
|
|
||
| _db.Orders.Add(order); | ||
| await _db.SaveChangesAsync(); | ||
|
|
||
| var result = order.ToViewModel(); | ||
| return Created($"/{order.Id}", result); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚩 Created response Location header will be wrong for gateway clients The Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Intentional, and as the finding notes this is the known prefix-stripping gateway trade-off (same as Notification's hardcoded internal paths). The task pins absolute service-root routing with the gateway stripping |
||
| } | ||
|
|
||
| [HttpGet("{id}")] | ||
| public IActionResult GetById(int id) | ||
| [HttpPut("/{id:int}")] | ||
| [ProducesResponseType(typeof(OrderVM), StatusCodes.Status200OK)] | ||
| [ProducesResponseType(StatusCodes.Status400BadRequest)] | ||
| [ProducesResponseType(StatusCodes.Status404NotFound)] | ||
| public async Task<IActionResult> Update(int id, [FromBody] OrderVM model) | ||
| { | ||
| // TODO: Implement — migrate logic from monolith | ||
| return Ok(new { service = "Order", id }); | ||
| if (!ModelState.IsValid) | ||
| return BadRequest(ModelState); | ||
|
|
||
| var order = await _db.Orders | ||
| .Include(o => o.OrderDetails) | ||
| .FirstOrDefaultAsync(o => o.Id == id); | ||
|
|
||
| if (order is null) | ||
| return NotFound(id); | ||
|
|
||
| model.ApplyTo(order); | ||
|
|
||
| var now = DateTime.UtcNow; | ||
| var userName = User.Identity?.Name; | ||
|
|
||
| _db.OrderDetails.RemoveRange(order.OrderDetails); | ||
| order.OrderDetails.Clear(); | ||
| foreach (var line in model.OrderDetails) | ||
| { | ||
| var detail = line.ToEntity(); | ||
| Stamp(detail, now, userName, isNew: true); | ||
| order.OrderDetails.Add(detail); | ||
| } | ||
|
|
||
| Stamp(order, now, userName, isNew: false); | ||
|
|
||
| await _db.SaveChangesAsync(); | ||
|
|
||
| return Ok(order.ToViewModel()); | ||
| } | ||
|
|
||
| [HttpDelete("/{id:int}")] | ||
| [ProducesResponseType(StatusCodes.Status204NoContent)] | ||
| [ProducesResponseType(StatusCodes.Status404NotFound)] | ||
| public async Task<IActionResult> Delete(int id) | ||
| { | ||
| var order = await _db.Orders | ||
| .Include(o => o.OrderDetails) | ||
| .FirstOrDefaultAsync(o => o.Id == id); | ||
|
|
||
| if (order is null) | ||
| return NotFound(id); | ||
|
|
||
| _db.Orders.Remove(order); | ||
| await _db.SaveChangesAsync(); | ||
|
|
||
| return NoContent(); | ||
| } | ||
|
|
||
| private static void Stamp(Order.Domain.Entities.BaseEntity entity, DateTime now, string? userName, bool isNew) | ||
| { | ||
| if (isNew) | ||
| { | ||
| entity.CreatedDate = now; | ||
| entity.CreatedBy = userName; | ||
| } | ||
|
|
||
| entity.UpdatedDate = now; | ||
| entity.UpdatedBy = userName; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| using Order.API.ViewModels; | ||
| using OrderEntity = Order.Domain.Entities.Order; | ||
| using OrderDetailEntity = Order.Domain.Entities.OrderDetail; | ||
|
|
||
| namespace Order.API.Mapping; | ||
|
|
||
| public static class OrderMappingExtensions | ||
| { | ||
| public static OrderVM ToViewModel(this OrderEntity entity) => new() | ||
| { | ||
| Id = entity.Id, | ||
| Discount = entity.Discount, | ||
| Comments = entity.Comments, | ||
| CustomerId = entity.CustomerId, | ||
| OrderDetails = entity.OrderDetails | ||
| .Select(d => d.ToViewModel()) | ||
| .ToList() | ||
| }; | ||
|
|
||
| public static OrderDetailVM ToViewModel(this OrderDetailEntity entity) => new() | ||
| { | ||
| Id = entity.Id, | ||
| ProductId = entity.ProductId, | ||
| Quantity = entity.Quantity, | ||
| UnitPrice = entity.UnitPrice, | ||
| Discount = entity.Discount | ||
| }; | ||
|
|
||
| public static OrderEntity ToEntity(this OrderVM vm) => new() | ||
| { | ||
| Discount = vm.Discount, | ||
| Comments = vm.Comments, | ||
| CustomerId = vm.CustomerId | ||
| }; | ||
|
|
||
| public static void ApplyTo(this OrderVM vm, OrderEntity entity) | ||
| { | ||
| entity.Discount = vm.Discount; | ||
| entity.Comments = vm.Comments; | ||
| entity.CustomerId = vm.CustomerId; | ||
| } | ||
|
|
||
| public static OrderDetailEntity ToEntity(this OrderDetailVM vm) => new() | ||
| { | ||
| ProductId = vm.ProductId, | ||
| Quantity = vm.Quantity, | ||
| UnitPrice = vm.UnitPrice, | ||
| Discount = vm.Discount | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| using System.ComponentModel.DataAnnotations; | ||
|
|
||
| namespace Order.API.ViewModels; | ||
|
|
||
| public class OrderVM | ||
| { | ||
| public int Id { get; set; } | ||
|
|
||
| public decimal Discount { get; set; } | ||
|
|
||
| [StringLength(500)] | ||
| public string? Comments { get; set; } | ||
|
|
||
| public int CustomerId { get; set; } | ||
|
|
||
| public List<OrderDetailVM> OrderDetails { get; set; } = []; | ||
| } | ||
|
|
||
| public class OrderDetailVM | ||
| { | ||
| public int Id { get; set; } | ||
|
|
||
| public int ProductId { get; set; } | ||
|
|
||
| public int Quantity { get; set; } | ||
|
|
||
| public decimal UnitPrice { get; set; } | ||
|
|
||
| public decimal Discount { get; set; } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| using System.ComponentModel.DataAnnotations; | ||
|
|
||
| namespace Order.Domain.Entities; | ||
|
|
||
| public abstract class BaseEntity | ||
| { | ||
| public int Id { get; set; } | ||
|
|
||
| [MaxLength(40)] | ||
| public string? CreatedBy { get; set; } | ||
|
|
||
| [MaxLength(40)] | ||
| public string? UpdatedBy { get; set; } | ||
|
|
||
| public DateTime CreatedDate { get; set; } | ||
|
|
||
| public DateTime UpdatedDate { get; set; } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚩 GetAll loads entire orders table without pagination
The
GetAllendpoint (OrderController.cs:33-36) executesToListAsync()over the fullOrderstable including allOrderDetails. Unlike the Notification service which acceptspageandpageSizequery parameters (NotificationController.cs:27), this endpoint has no pagination. With a large dataset this will cause high memory usage and slow responses. This is a design concern worth addressing before production use.Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Valid design concern, but out of scope for this carve-out task, which pins the resource controller surface (
GET /→ list). No functional bug here. Pagination (matching Notification'spage/pageSize) would be a sensible follow-up before production; leaving it out to keep this PR focused on the decoupling + auth + persistence gate.