-
Notifications
You must be signed in to change notification settings - Fork 0
Customer service: extract Customer bounded context from monolith (.NET 10) #56
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
fdca61e
6384f31
9c24b43
653e304
79d2426
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,165 @@ | ||
| using Customer.API.ViewModels; | ||
| using Customer.Infrastructure.Data; | ||
| using Microsoft.AspNetCore.Authorization; | ||
| using Microsoft.AspNetCore.Mvc; | ||
| using Microsoft.EntityFrameworkCore; | ||
| using CustomerEntity = Customer.Domain.Entities.Customer; | ||
| using GenderEnum = Customer.Domain.Entities.Gender; | ||
|
|
||
| namespace Customer.API.Controllers; | ||
|
|
||
| [ApiController] | ||
| [Route("api/[controller]")] | ||
| [Authorize] | ||
| [Produces("application/json")] | ||
| public class CustomerController : ControllerBase | ||
| { | ||
| private readonly CustomerDbContext _dbContext; | ||
| private readonly ILogger<CustomerController> _logger; | ||
|
|
||
| public CustomerController(ILogger<CustomerController> logger) | ||
| public CustomerController(CustomerDbContext dbContext, ILogger<CustomerController> logger) | ||
| { | ||
| _dbContext = dbContext; | ||
| _logger = logger; | ||
| } | ||
|
|
||
| [HttpGet] | ||
| public IActionResult GetAll() | ||
| // External: GET /api/customers -> service GET / | ||
| [HttpGet("/")] | ||
| [ProducesResponseType(typeof(IEnumerable<CustomerVM>), StatusCodes.Status200OK)] | ||
| [ProducesResponseType(StatusCodes.Status401Unauthorized)] | ||
| public async Task<ActionResult<IEnumerable<CustomerVM>>> GetAll() | ||
| { | ||
| // TODO: Implement — migrate logic from monolith's CustomerController | ||
| return Ok(new { service = "Customer", status = "scaffold" }); | ||
| var customers = await _dbContext.Customers | ||
| .OrderBy(c => c.Name) | ||
| .ToListAsync(); | ||
|
|
||
| return Ok(customers.Select(ToViewModel)); | ||
| } | ||
|
|
||
| // External: GET /api/customers/{id} -> service GET /{id} | ||
| [HttpGet("/{id:int}")] | ||
| [ProducesResponseType(typeof(CustomerVM), StatusCodes.Status200OK)] | ||
| [ProducesResponseType(StatusCodes.Status404NotFound)] | ||
| [ProducesResponseType(StatusCodes.Status401Unauthorized)] | ||
| public async Task<ActionResult<CustomerVM>> GetById(int id) | ||
| { | ||
| var customer = await _dbContext.Customers.FindAsync(id); | ||
| if (customer == null) | ||
| return NotFound(); | ||
|
|
||
| return Ok(ToViewModel(customer)); | ||
| } | ||
|
|
||
| // External: POST /api/customers -> service POST / | ||
| [HttpPost("/")] | ||
| [ProducesResponseType(typeof(CustomerVM), StatusCodes.Status201Created)] | ||
| [ProducesResponseType(StatusCodes.Status400BadRequest)] | ||
| [ProducesResponseType(StatusCodes.Status401Unauthorized)] | ||
| public async Task<ActionResult<CustomerVM>> Create([FromBody] CustomerVM model) | ||
| { | ||
| if (string.IsNullOrWhiteSpace(model.Name)) | ||
| ModelState.AddModelError(nameof(model.Name), "Customer name cannot be empty"); | ||
| if (string.IsNullOrWhiteSpace(model.Email)) | ||
| ModelState.AddModelError(nameof(model.Email), "Email cannot be empty"); | ||
| if (string.IsNullOrWhiteSpace(model.Gender)) | ||
| ModelState.AddModelError(nameof(model.Gender), "Gender cannot be empty"); | ||
| else if (!Enum.TryParse<GenderEnum>(model.Gender, ignoreCase: true, out var parsedGender) || !Enum.IsDefined(parsedGender)) | ||
| ModelState.AddModelError(nameof(model.Gender), "Invalid gender value"); | ||
| if (!ModelState.IsValid) | ||
| return BadRequest(ModelState); | ||
|
Comment on lines
+63
to
+68
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. 🚩 Gender validation allows 'None' as a valid input despite requiring a non-empty value The validation at 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 — keeping |
||
|
|
||
| var customer = new CustomerEntity | ||
| { | ||
| Name = model.Name!, | ||
| Email = model.Email!, | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
|
||
| PhoneNumber = model.PhoneNumber, | ||
| Address = model.Address, | ||
| City = model.City, | ||
| Gender = ParseGender(model.Gender), | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
|
||
| CreatedBy = CurrentUser(), | ||
| UpdatedBy = CurrentUser(), | ||
| CreatedDate = DateTime.UtcNow, | ||
| UpdatedDate = DateTime.UtcNow | ||
| }; | ||
|
|
||
| _dbContext.Customers.Add(customer); | ||
| await _dbContext.SaveChangesAsync(); | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
|
||
|
|
||
| var vm = ToViewModel(customer); | ||
| return CreatedAtAction(nameof(GetById), new { id = customer.Id }, vm); | ||
|
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. 🚩 CreatedAtAction Location header will be incorrect 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. Acknowledged — this is intentional given the decoupling constraints. The service is deliberately prefix-agnostic (the gateway owns
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-resource Location URL points to the wrong path, giving clients a 404 when they follow it The Location header in the "customer created" response is built from a root-level route ( Impact: Any REST client that follows the standard Location header from a 201 Created response will receive a 404 from the API gateway. Root cause: gateway prefix stripped on inbound but never restored on outbound URLsThe YARP API gateway matches requests at The gateway has no response transform to rewrite the Location header, and the service does not use Compare with the Notification service ( Prompt for agentsWas 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. Acknowledged. The routing shape here (absolute root routes + gateway |
||
| } | ||
|
|
||
| // External: PUT /api/customers/{id} -> service PUT /{id} | ||
| [HttpPut("/{id:int}")] | ||
| [ProducesResponseType(StatusCodes.Status204NoContent)] | ||
| [ProducesResponseType(StatusCodes.Status400BadRequest)] | ||
| [ProducesResponseType(StatusCodes.Status404NotFound)] | ||
| [ProducesResponseType(StatusCodes.Status401Unauthorized)] | ||
| public async Task<IActionResult> Update(int id, [FromBody] CustomerVM model) | ||
| { | ||
| var customer = await _dbContext.Customers.FindAsync(id); | ||
| if (customer == null) | ||
| return NotFound(); | ||
|
|
||
| if (string.IsNullOrWhiteSpace(model.Name)) | ||
| ModelState.AddModelError(nameof(model.Name), "Customer name cannot be empty"); | ||
| if (string.IsNullOrWhiteSpace(model.Email)) | ||
| ModelState.AddModelError(nameof(model.Email), "Email cannot be empty"); | ||
| if (string.IsNullOrWhiteSpace(model.Gender)) | ||
| ModelState.AddModelError(nameof(model.Gender), "Gender cannot be empty"); | ||
| else if (!Enum.TryParse<GenderEnum>(model.Gender, ignoreCase: true, out var parsedGender) || !Enum.IsDefined(parsedGender)) | ||
| ModelState.AddModelError(nameof(model.Gender), "Invalid gender value"); | ||
| if (!ModelState.IsValid) | ||
| return BadRequest(ModelState); | ||
|
|
||
| customer.Name = model.Name!; | ||
| customer.Email = model.Email!; | ||
| customer.PhoneNumber = model.PhoneNumber; | ||
| customer.Address = model.Address; | ||
| customer.City = model.City; | ||
| customer.Gender = ParseGender(model.Gender); | ||
| customer.UpdatedBy = CurrentUser(); | ||
| customer.UpdatedDate = DateTime.UtcNow; | ||
|
|
||
| await _dbContext.SaveChangesAsync(); | ||
| return NoContent(); | ||
| } | ||
|
|
||
| // External: DELETE /api/customers/{id} -> service DELETE /{id} | ||
| [HttpDelete("/{id:int}")] | ||
| [ProducesResponseType(StatusCodes.Status204NoContent)] | ||
| [ProducesResponseType(StatusCodes.Status404NotFound)] | ||
| [ProducesResponseType(StatusCodes.Status401Unauthorized)] | ||
| public async Task<IActionResult> Delete(int id) | ||
| { | ||
| var customer = await _dbContext.Customers.FindAsync(id); | ||
| if (customer == null) | ||
| return NotFound(); | ||
|
|
||
| _dbContext.Customers.Remove(customer); | ||
| await _dbContext.SaveChangesAsync(); | ||
| return NoContent(); | ||
| } | ||
|
|
||
| [HttpGet("{id}")] | ||
| public IActionResult GetById(int id) | ||
| private static CustomerVM ToViewModel(CustomerEntity c) => new() | ||
| { | ||
| Id = c.Id, | ||
| Name = c.Name, | ||
| Email = c.Email, | ||
| PhoneNumber = c.PhoneNumber, | ||
| Address = c.Address, | ||
| City = c.City, | ||
| Gender = c.Gender.ToString() | ||
| }; | ||
|
|
||
| private static GenderEnum ParseGender(string? value) => | ||
| Enum.TryParse<GenderEnum>(value, ignoreCase: true, out var gender) ? gender : GenderEnum.None; | ||
|
|
||
| // CreatedBy/UpdatedBy column is varchar(40); truncate the JWT identity to fit. | ||
| private string? CurrentUser() | ||
| { | ||
| // TODO: Implement — migrate logic from monolith | ||
| return Ok(new { service = "Customer", id }); | ||
| var name = User.Identity?.Name; | ||
| if (string.IsNullOrEmpty(name)) | ||
| return null; | ||
| return name.Length > 40 ? name[..40] : name; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,24 +1,93 @@ | ||
| using System.Text; | ||
| using Customer.Infrastructure.Data; | ||
| using Microsoft.AspNetCore.Authentication.JwtBearer; | ||
| using Microsoft.EntityFrameworkCore; | ||
| using Microsoft.IdentityModel.Tokens; | ||
| using Microsoft.OpenApi.Models; | ||
|
|
||
| var builder = WebApplication.CreateBuilder(args); | ||
|
|
||
| builder.Services.AddControllers(); | ||
| builder.Services.AddEndpointsApiExplorer(); | ||
| builder.Services.AddSwaggerGen(); | ||
| builder.Services.AddSwaggerGen(options => | ||
| { | ||
| options.AddSecurityDefinition("Bearer", new OpenApiSecurityScheme | ||
| { | ||
| Name = "Authorization", | ||
| Type = SecuritySchemeType.Http, | ||
| Scheme = "bearer", | ||
| BearerFormat = "JWT", | ||
| In = ParameterLocation.Header, | ||
| Description = "Enter the JWT bearer token issued by the Identity service." | ||
| }); | ||
| options.AddSecurityRequirement(new OpenApiSecurityRequirement | ||
| { | ||
| { | ||
| new OpenApiSecurityScheme | ||
| { | ||
| Reference = new OpenApiReference { Type = ReferenceType.SecurityScheme, Id = "Bearer" } | ||
| }, | ||
| Array.Empty<string>() | ||
| } | ||
| }); | ||
| }); | ||
| builder.Services.AddHealthChecks(); | ||
|
|
||
| builder.Services.AddDbContext<CustomerDbContext>(options => | ||
| options.UseNpgsql(builder.Configuration.GetConnectionString("DefaultConnection"))); | ||
|
|
||
| var jwtSection = builder.Configuration.GetSection("Jwt"); | ||
| builder.Services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme) | ||
| .AddJwtBearer(options => | ||
| { | ||
| options.MapInboundClaims = false; | ||
| options.TokenValidationParameters = new TokenValidationParameters | ||
| { | ||
| ValidateIssuer = true, | ||
| ValidateAudience = true, | ||
| ValidateLifetime = true, | ||
| ValidateIssuerSigningKey = true, | ||
| ValidIssuer = jwtSection["Issuer"], | ||
| ValidAudience = jwtSection["Audience"], | ||
| IssuerSigningKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(jwtSection["Key"]!)), | ||
| NameClaimType = "name", | ||
| RoleClaimType = "role" | ||
| }; | ||
| }); | ||
| builder.Services.AddAuthorization(); | ||
|
|
||
| var app = builder.Build(); | ||
|
|
||
| using (var scope = app.Services.CreateScope()) | ||
| { | ||
| var db = scope.ServiceProvider.GetRequiredService<CustomerDbContext>(); | ||
| var logger = scope.ServiceProvider.GetRequiredService<ILoggerFactory>().CreateLogger("Startup"); | ||
|
|
||
| const int maxAttempts = 12; | ||
| for (var attempt = 1; ; attempt++) | ||
| { | ||
| try | ||
| { | ||
| db.Database.EnsureCreated(); | ||
|
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. 🚩 EnsureCreated used instead of EF Core migrations for schema management The startup code at 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. Acknowledged and intentional for this PR's scope. |
||
| break; | ||
| } | ||
| catch (Exception ex) when (attempt < maxAttempts) | ||
| { | ||
| logger.LogWarning(ex, "Database not ready (attempt {Attempt}/{Max}); retrying in 5s...", attempt, maxAttempts); | ||
| Thread.Sleep(TimeSpan.FromSeconds(5)); | ||
| } | ||
| } | ||
| } | ||
|
Comment on lines
+67
to
+80
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. 🚩 Thread.Sleep blocks the startup thread but is acceptable in this context The database retry loop at 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. Agreed it's acceptable here — it runs once at startup in the synchronous top-level context before the app serves traffic. The bound is 12 × 5s only in the pathological case where Postgres never becomes reachable; in practice it connects on the first retry (compose |
||
|
|
||
| if (app.Environment.IsDevelopment()) | ||
| { | ||
| app.UseSwagger(); | ||
| app.UseSwaggerUI(); | ||
| } | ||
|
|
||
| app.UseAuthentication(); | ||
| app.UseAuthorization(); | ||
|
|
||
| app.MapControllers(); | ||
| app.MapHealthChecks("/healthz"); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| namespace Customer.API.ViewModels; | ||
|
|
||
| public class CustomerVM | ||
| { | ||
| public int Id { get; set; } | ||
| public string? Name { get; set; } | ||
| public string? Email { get; set; } | ||
| public string? PhoneNumber { get; set; } | ||
| public string? Address { get; set; } | ||
| public string? City { get; set; } | ||
| public string? Gender { get; set; } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| using System.ComponentModel.DataAnnotations; | ||
|
|
||
| namespace Customer.Domain.Entities; | ||
|
|
||
| public class BaseEntity | ||
| { | ||
| public int Id { get; set; } | ||
|
|
||
| [MaxLength(40)] | ||
| public string? CreatedBy { get; set; } | ||
|
|
||
| [MaxLength(40)] | ||
| public string? UpdatedBy { get; set; } | ||
|
|
||
| public DateTime UpdatedDate { get; set; } | ||
|
|
||
| public DateTime CreatedDate { get; set; } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| namespace Customer.Domain.Entities; | ||
|
|
||
| public class Customer : BaseEntity | ||
| { | ||
| public required string Name { get; set; } | ||
| public required string Email { get; set; } | ||
| public string? PhoneNumber { get; set; } | ||
| public string? Address { get; set; } | ||
| public string? City { get; set; } | ||
| public Gender Gender { get; set; } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| namespace Customer.Domain.Entities; | ||
|
|
||
| public enum Gender | ||
| { | ||
| None, | ||
| Female, | ||
| Male | ||
| } |
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.
🚩 Controller drops [Route] attribute, diverging from every other service's pattern
All other controllers in the repo (Identity, Order, Product, Notification) use
[Route("api/[controller]")]with relative action routes. This controller removes the[Route]attribute and uses absolute routes with leading/(e.g.[HttpGet("/")]). The comments explain this is intentional for gateway prefix stripping, but it creates an inconsistency across services that could confuse contributors. The Notification service (src/Services/Notification/Notification.API/Controllers/NotificationController.cs:9) keeps its controller-level[Route], suggesting no repo-wide decision to move to root routes.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.
This is intentional and required by the pinned routing convention for this service: the gateway matches
/api/customers/{**catch-all}and strips the prefix viaPathRemovePrefix, so the controller must sit at the service root with absolute routes ([HttpGet("/")],[HttpGet("/{id:int}")], etc.). Using[Route("api/[controller]")]here would mean the service expects/api/customers/...while the gateway forwards/..., breaking routing. The inline comments document this so contributors understand the divergence is deliberate.