Customer service: extract Customer bounded context from monolith (.NET 10) by devin-ai-integration[bot] · Pull Request #56 · Cognition-Partner-Workshops/quickapp-microservices

devin-ai-integration · 2026-06-26T04:38:15Z

Summary

Implements the standalone Customer microservice by carving the Customer bounded context out of the monolith. Real EF/Npgsql persistence + full CRUD, JWT bearer auth on every endpoint, and Swagger. All changes are scoped to src/Services/Customer/**.

Database-per-service decoupling

The monolith Customer entity has an ICollection<Order> Orders navigation and the monolith service eager-loads Orders → OrderDetails → Product/Cashier. This service owns only customer data: the Orders nav is dropped entirely, and the forbidden symbols Order, OrderDetail, ApplicationUser, Cashier appear zero times in src/Services/Customer/** (the only substring hit is the LINQ .OrderBy(...)). BaseEntity (int Id + CreatedBy/UpdatedBy [MaxLength 40] + CreatedDate/UpdatedDate) and the Gender enum are recreated inside Customer.Domain rather than referenced from the monolith.

Ported persistence constraints (verbatim from monolith `ApplicationDbContext`)

Customer.Name        IsRequired() + HasMaxLength(100) + HasIndex(c => c.Name)
Customer.Email       HasMaxLength(100)
Customer.PhoneNumber IsUnicode(false) + HasMaxLength(30)   // IsUnicode is a no-op on Postgres
Customer.City        HasMaxLength(50)
ToTable("AppCustomers")

Schema is created on boot via EnsureCreated() (acceptable per scope; long-term follow-up: switch to Database.Migrate() + an initial migration). Connection string from ConnectionStrings:DefaultConnection.

Routing (gateway strips `/api/customers` prefix)

The YARP route uses PathRemovePrefix /api/customers, so the resource controller is at the service root via absolute routes:

external GET    /api/customers        -> service GET    /
external GET    /api/customers/{id}   -> service GET    /{id:int}
external POST   /api/customers        -> service POST   /
external PUT    /api/customers/{id}   -> service PUT    /{id:int}
external DELETE /api/customers/{id}   -> service DELETE /{id:int}
health /healthz (external /api/customers/healthz)

CustomerVM mirrors the monolith DTO (Id, Name, Email, PhoneNumber, Address, City, Gender-as-string) minus the Orders collection; entity↔DTO mapping is manual.

Input validation (Create/Update)

Returns 400 (not a 500/silent corruption) for: blank Name, blank Email (entity column is NOT NULL), blank Gender, and unrecognized Gender — including out-of-range numeric strings, guarded by Enum.TryParse(...) || !Enum.IsDefined(...).

Auth — canonical JWT contract (Identity service PR #48)

Every endpoint is [Authorize] (unauthenticated → 401). Added Microsoft.AspNetCore.Authentication.JwtBearer (10.*) and a Jwt section to appsettings.json (Key/Issuer=quickapp-identity/Audience=quickapp). AddJwtBearer validates issuer/audience/lifetime/signing-key (HS256, SymmetricSecurityKey over UTF8 bytes of Jwt:Key), with MapInboundClaims = false, NameClaimType = "name", RoleClaimType = "role". app.UseAuthentication(); app.UseAuthorization(); wired in.

Scaffold bug fixed

Customer.API.csproj referenced the Shared projects via ..\..\Shared\... (resolves to src/Services/Shared, which doesn't exist). The Shared refs are unused by this service, so they were dropped, and the corresponding COPY lines were removed from the Dockerfile so the container build matches.

Startup resilience

EnsureCreated() is wrapped in a bounded retry loop (12 × 5s) so the container survives Postgres not being ready yet on a cold docker compose up.

Validation (all gates pass)

cd src && dotnet build Services/Customer/Customer.API/Customer.API.csproj -c Release → succeeds.
Forbidden-symbol grep over the service → zero references to Order/OrderDetail/ApplicationUser/Cashier (only .OrderBy LINQ).
Brought up postgres + customer-service + api-gateway via docker compose. Through the gateway at http://localhost:5000:
- GET /api/customers/healthz → 200
- GET /api/customers (no token) → 401; bad token → 401
- valid HS256 token (matching the contract) → GET 200, POST 201 with real persistence, row read back via GET.
- invalid input → 400 (missing email, missing/unknown/numeric gender).
AppCustomers table confirmed created in customerdb (psql \d "AppCustomers"): Name varchar(100) NOT NULL, Email varchar(100), PhoneNumber varchar(30), City varchar(50), PK on Id, index IX_AppCustomers_Name.
Added self-contained src/Services/Customer/e2e-smoke.sh asserting health 200 + unauth 401 through the gateway (exits non-zero on failure).

Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/0bf0f669664e4b2c88c64d8132bc12f8
Requested by: @mbatchelor81

…ll CRUD

devin-ai-integration · 2026-06-26T04:38:18Z

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

Disable automatic comment, CI, and merge conflict monitoring

devin-ai-integration

Devin Review found 3 potential issues.

View 3 additional findings in Devin Review.

devin-ai-integration · 2026-06-26T04:41:47Z

+        await _dbContext.SaveChangesAsync();
+
+        var vm = ToViewModel(customer);
+        return CreatedAtAction(nameof(GetById), new { id = customer.Id }, vm);


🚩 CreatedAtAction Location header will be incorrect for gateway clients

The CreatedAtAction(nameof(GetById), new { id = customer.Id }, vm) call at line 82 generates a Location header pointing to the internal service URL (e.g., /5). Clients calling through the API gateway expect /api/customers/5. YARP does not automatically rewrite response Location headers. This means the 201 Created response's Location header is technically incorrect for external consumers. Most REST clients use the response body rather than the Location header, so impact is low, but it violates HTTP 201 semantics for gateway-routed requests.

Was this helpful? React with 👍 or 👎 to provide feedback.

Acknowledged — this is intentional given the decoupling constraints. The service is deliberately prefix-agnostic (the gateway owns /api/customers and strips it via PathRemovePrefix), so the service cannot know its external base path without coupling it back to the gateway. The Location header will be the service-relative /{id}; the 201 body carries the full resource (including id), which is what clients consume here. Leaving as-is to preserve the database-per-service / prefix-independence design rather than hard-coding the external prefix.

devin-ai-integration · 2026-06-26T04:41:48Z

+    for (var attempt = 1; ; attempt++)
+    {
+        try
+        {
+            db.Database.EnsureCreated();
+            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));
+        }
+    }
+}


🚩 Thread.Sleep blocks the startup thread but is acceptable in this context

The database retry loop at src/Services/Customer/Customer.API/Program.cs:77 uses Thread.Sleep(TimeSpan.FromSeconds(5)) rather than await Task.Delay(...). Since this runs during application startup (before the app handles requests) and is in a synchronous top-level context, blocking the thread is acceptable. However, in containerized environments with health check probes on tight timeouts, a 60-second (12 × 5s) maximum block before the app starts could cause orchestrators (e.g., Kubernetes) to kill the pod. The /healthz endpoint won't respond until after this loop completes.

Was this helpful? React with 👍 or 👎 to provide feedback.

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 depends_on brings Postgres up first). Keeping Thread.Sleep for simplicity rather than introducing an async startup path. If we later add k8s liveness/readiness probes with tight timeouts, this loop would move behind a readiness gate.

… email)

…g None

devin-ai-integration

Devin Review found 2 new potential issues.

View 3 additional findings in Devin Review.

devin-ai-integration · 2026-06-26T04:54:35Z

+    {
+        try
+        {
+            db.Database.EnsureCreated();


🚩 EnsureCreated used instead of EF Core migrations for schema management

The startup code at Program.cs:71 uses db.Database.EnsureCreated() which creates the database and schema if they don't exist, but will NOT update an existing database when the schema changes (e.g., new columns or entities are added). For a service that's expected to evolve, this means schema changes will silently not be applied, requiring manual intervention or a switch to db.Database.Migrate(). This may be acceptable for initial scaffolding but should be tracked for future work.

Was this helpful? React with 👍 or 👎 to provide feedback.

Acknowledged and intentional for this PR's scope. EnsureCreated() is an explicitly-accepted option for bootstrapping this service's schema (the alternative, an EF migration applied at startup via Migrate(), is the preferred long-term approach). Switching to migrations means generating and committing a migrations assembly, which is broader than this carve-out. Tracking it as follow-up: replace EnsureCreated() with Database.Migrate() + an initial migration once the schema starts evolving.

devin-ai-integration

Devin Review found 4 new potential issues.

View 2 additional findings in Devin Review.

devin-ai-integration · 2026-06-26T05:04:47Z

+        await _dbContext.SaveChangesAsync();
+
+        var vm = ToViewModel(customer);
+        return CreatedAtAction(nameof(GetById), new { id = customer.Id }, vm);


🟡 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 (CreatedAtAction(nameof(GetById), ...) at src/Services/Customer/Customer.API/Controllers/CustomerController.cs:86) without the gateway path prefix, so clients following that URL get a 404.

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 URLs

The YARP API gateway matches requests at /api/customers/{**catch-all} and applies PathRemovePrefix: /api/customers (src/ApiGateway/appsettings.json:19), so the Customer service sees requests at /. The GetById action has an absolute route template /{id:int} (CustomerController.cs:39), so CreatedAtAction generates a Location path of /{id} (e.g. /5) rather than /api/customers/5.

The gateway has no response transform to rewrite the Location header, and the service does not use UseForwardedHeaders or set a PathBase. Consequently, the Location header in the 201 response points to a URL like http://gateway:5000/5, which matches no gateway route and returns 404.

Compare with the Notification service (src/Services/Notification/Notification.API/Controllers/NotificationController.cs:9) which uses [Route("api/[controller]")] and relative routes, generating self-consistent URLs (though that service has its own singular/plural mismatch).

Prompt for agents

The CreatedAtAction call generates a Location URL using the service's internal route (/{id}), but the YARP gateway strips the /api/customers prefix from inbound requests and does not add it back to outbound response headers. This means the Location header points to an unreachable path (e.g. /5 instead of /api/customers/5). There are several approaches to fix this: 1. Add UseForwardedHeaders middleware in Program.cs and configure the gateway to pass X-Forwarded-Prefix (or X-Forwarded-PathBase). Then ASP.NET Core will set PathBase correctly and URL generation will include the prefix. 2. Add a YARP response transform (ResponseHeaderRoutePrefix or a custom transform) to rewrite Location headers in responses. 3. Re-add [Route("api/customers")] to the CustomerController (matching the gateway prefix pattern) and remove the leading / from all action route templates, so that URLs are generated with the full /api/customers/{id} path. This would mean the gateway's PathRemovePrefix transform is no longer needed for this service (or should be removed). Option 3 is simplest and most consistent with other services in the repo (e.g. NotificationController uses [Route("api/[controller]")]).

Was this helpful? React with 👍 or 👎 to provide feedback.

Acknowledged. The routing shape here (absolute root routes + gateway PathRemovePrefix) is a pinned system convention for this service, so option 3 (re-adding [Route("api/customers")]) is out of scope — it would reintroduce the prefix the gateway is configured to strip. The service is intentionally prefix-agnostic and cannot synthesize the external base path without coupling back to the gateway. The Location header is therefore service-relative (/{id}); the 201 body carries the full resource (including id), which is what clients consume. If we later want a correct external Location, the right place is a gateway-side response transform (or X-Forwarded-Prefix + UsePathBase), not a change to this service's routing.

devin-ai-integration · 2026-06-26T05:04:48Z

+        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);


🚩 Gender validation allows 'None' as a valid input despite requiring a non-empty value

The validation at src/Services/Customer/Customer.API/Controllers/CustomerController.cs:63-66 rejects null/whitespace Gender but accepts "None" (case-insensitive) since it's a defined enum value (Gender.None at src/Services/Customer/Customer.Domain/Entities/Gender.cs:5). This means a user can explicitly set gender to the 'unspecified' sentinel value, which may defeat the purpose of the non-empty check. Whether this is intentional depends on product requirements — if None is meant only as a default/unset marker, it should be excluded from valid inputs.

Was this helpful? React with 👍 or 👎 to provide feedback.

Intentional — keeping None as an accepted value. None is a first-class defined member of the Gender enum carried over verbatim from the monolith (None/Female/Male), and it's the domain's "unspecified" value rather than an invalid one. The not-empty + IsDefined checks exist to reject blank/garbage/out-of-range input, not to forbid None. Excluding None would diverge from the source-of-truth domain model.

devin-ai-integration · 2026-06-26T05:04:49Z

 [ApiController]
-[Route("api/[controller]")]
+[Authorize]
+[Produces("application/json")]
 public class CustomerController : ControllerBase


🚩 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.

This is intentional and required by the pinned routing convention for this service: the gateway matches /api/customers/{**catch-all} and strips the prefix via PathRemovePrefix, 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.

…tity

Customer service: port entity/persistence from monolith, JWT auth, fu…

fdca61e

…ll CRUD

mbatchelor81 self-assigned this Jun 26, 2026

devin-ai-integration Bot commented Jun 26, 2026

View reviewed changes

Validate Email in Create/Update (return 400 instead of 500 on missing…

6384f31

… email)

This comment was marked as resolved.

Sign in to view

Reject unrecognized Gender values with 400 instead of silently storin…

9c24b43

…g None

devin-ai-integration Bot commented Jun 26, 2026

View reviewed changes

Reject out-of-range numeric Gender values via Enum.IsDefined

653e304

devin-ai-integration Bot commented Jun 26, 2026

View reviewed changes

Populate CreatedBy/UpdatedBy audit fields from authenticated JWT iden…

79d2426

…tity

Uh oh!

Conversation

devin-ai-integration Bot commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Database-per-service decoupling

Ported persistence constraints (verbatim from monolith ApplicationDbContext)

Routing (gateway strips /api/customers prefix)

Input validation (Create/Update)

Auth — canonical JWT contract (Identity service PR #48)

Scaffold bug fixed

Startup resilience

Validation (all gates pass)

Uh oh!

devin-ai-integration Bot commented Jun 26, 2026

🤖 Devin AI Engineer

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

devin-ai-integration Bot Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

devin-ai-integration Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

devin-ai-integration Bot commented Jun 26, 2026 •

edited

Loading

Ported persistence constraints (verbatim from monolith `ApplicationDbContext`)

Routing (gateway strips `/api/customers` prefix)

devin-ai-integration Bot Jun 26, 2026 •

edited

Loading