Extract Product bounded context into standalone Product microservice by devin-ai-integration[bot] · Pull Request #57 · Cognition-Partner-Workshops/quickapp-microservices

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

Summary

Implements the Product service scaffold at src/Services/Product/ as a standalone .NET 10 microservice carved out of the QuickApp monolith. Entities, persistence config, and DTO are ported from quickapp-monolith; the monolith itself is untouched. All endpoints require a JWT validated verbatim against the canonical Identity contract, and the service sits behind the existing /api/products gateway route.

Decoupling

The Product bounded context is fully self-contained — zero references to Order/OrderDetail anywhere in the service. The monolith's Product.OrderDetails navigation was dropped; the self-referential Parent/Children hierarchy and the owned ProductCategory relationship are kept.

Domain (`Product.Domain`)

Product and ProductCategory mirror the monolith verbatim except OrderDetails is removed. Keeps ParentId/Parent/Children and ProductCategoryId/ProductCategory, using required per conventions.
A local BaseEntity (int Id, CreatedBy/UpdatedBy [MaxLength(40)], CreatedDate/UpdatedDate) is recreated inside the service — it does not reference the monolith.
The root namespace segment collides with the Product type name, so infra/API reference it via using ProductEntity = Product.Domain.Product;.

Persistence (`Product.Infrastructure.ProductDbContext`, Npgsql)

Fluent config ported verbatim:

Product.Name      IsRequired + HasMaxLength(100) + HasIndex(p => p.Name)
Product.Description HasMaxLength(500)
Product.Icon      IsUnicode(false) + HasMaxLength(256)   // IsUnicode no-op on PG, kept
Product.BuyingPrice / SellingPrice  HasPrecision(18,2)
Product.Parent -> Children  OnDelete(Restrict)
-> ToTable("AppProducts")
ProductCategory.Name IsRequired + HasMaxLength(100); Description HasMaxLength(500)
-> ToTable("AppProductCategories")

Schema is created on boot via EnsureCreated() (with a short connect-retry loop for compose startup ordering). Connection string read from ConnectionStrings:DefaultConnection.

API (`Product.API`)

ProductVM mirrors the monolith DTO (flattened ProductCategoryName); manual entity↔DTO mapping.
Full CRUD controller with real EF persistence, [ProducesResponseType], Swagger (with Bearer security scheme). Optional GET /categories endpoint. Create/Update resolve ProductCategory by name (find-or-create) to satisfy the required nav.
Auth: controller-level [Authorize] → unauthenticated requests get 401. JWT bearer wired exactly per the canonical contract (HS256; MapInboundClaims=false; NameClaimType=name, RoleClaimType=role; validate issuer/audience/lifetime/signing-key against Jwt:Key|Issuer|Audience added to appsettings.json). app.UseAuthentication(); app.UseAuthorization(); added. NuGet Microsoft.AspNetCore.Authentication.JwtBearer (10.*) added.
Routing: gateway strips the /api/products prefix, so the controller uses absolute routes ([HttpGet("/")], [HttpGet("/{id:int}")], …) → external GET /api/products maps to service GET /. Health remains at /healthz (open, no token).

Scaffold bug fix

Product.API.csproj referenced Shared via ..\..\Shared\... (resolves to non-existent src/Services/Shared, MSB9008 warning). Fixed to ..\..\..\Shared\.... Dockerfile COPY lines are context-root-relative and already correct — left unchanged.

Validation gate (all passing)

cd src && dotnet build Services/Product/Product.API/Product.API.csproj -c Release → succeeds, 0 warnings.
Grep of src/Services/Product for Order/OrderDetail → zero refs.
docker compose up --no-deps postgres product-service api-gateway (only these three; --no-deps avoids the unrelated broken notification-service build pulled in via the gateway's depends_on):
- GET /api/products/healthz → 200
- GET /api/products (no token) → 401
- GET /api/products with valid canonical HS256 token → 200; tampered signature → 401
- POST /api/products (valid token) → 201, round-trips via GET /api/products
- Tables AppProducts and AppProductCategories created in productdb (verified via psql \dt), with IX_AppProducts_Name, numeric(18,2) price columns, and FK_AppProducts_AppProducts_ParentId ... ON DELETE RESTRICT.
src/Services/Product/e2e-smoke.sh asserts health 200 + unauth 401 through the gateway and exits non-zero on failure.

Scope

Changes are confined to src/Services/Product/**. The monolith, gateway appsettings.json, and docker-compose.yml were not modified.

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

- Port Product/ProductCategory entities + BaseEntity into Product.Domain (drop OrderDetails nav; keep Parent/Children + ProductCategory) - Port verbatim fluent config into ProductDbContext (AppProducts/AppProductCategories, Name index, 18,2 precision, Parent->Children Restrict); EnsureCreated on boot - Add ProductVM DTO + full CRUD controller with real EF persistence, [Authorize], [ProducesResponseType], Swagger - Wire JWT bearer (HS256) per canonical Identity contract; UseAuthentication/UseAuthorization - Use absolute routes so gateway prefix-stripping maps /api/products -> / - Fix scaffold Shared project reference path (..\..\Shared -> ..\..\..\Shared) - Add e2e-smoke.sh (health 200 + unauth 401 through gateway)

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

🤖 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:44:28Z

+    [HttpGet("/")]
+    [ProducesResponseType(typeof(IEnumerable<ProductVM>), StatusCodes.Status200OK)]


🚩 Absolute route templates bypass controller-level route prefix convention

All action methods use absolute route templates starting with / (e.g., [HttpGet("/")], [HttpGet("/{id:int}")]). This is a deliberate departure from the [Route("api/[controller]")] convention used by every other controller in the solution (src/Services/Identity/Identity.API/Controllers/IdentityController.cs:6, src/Services/Customer/Customer.API/Controllers/CustomerController.cs:6, src/Services/Order/Order.API/Controllers/OrderController.cs:6). The design works correctly because the YARP gateway strips /api/products via PathRemovePrefix (src/ApiGateway/appsettings.json:29), so the service receives root-relative paths. However, this means the Product service's Swagger UI will show routes at /, /{id}, /categories rather than under a prefix — making direct-to-service testing less intuitive and establishing a different pattern from the other microservices that will need to be reconciled when they are similarly migrated.

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

Intentional and pinned by the carve-out spec. The gateway route Match /api/products/{**catch-all} + PathRemovePrefix /api/products means the service receives the prefix stripped, so external GET /api/products arrives as GET /. Absolute routes ([HttpGet("/")], [HttpGet("/{id:int}")]) are required to map the resource controller to the service root. The other services still carry the placeholder [Route("api/[controller]")] from the scaffold and haven't been wired to their gateway contracts yet; reconciling that convention is part of their own carve-outs. Leaving as-is for this PR.

devin-ai-integration · 2026-06-26T04:44:29Z

+        modelBuilder.Entity<ProductCategory>(entity =>
+        {
+            entity.Property(p => p.Name).IsRequired().HasMaxLength(100);
+            entity.Property(p => p.Description).HasMaxLength(500);
+            entity.ToTable("AppProductCategories");
+        });


🚩 No unique constraint on ProductCategory.Name allows duplicate categories under concurrency

The ResolveCategoryAsync method (src/Services/Product/Product.API/Controllers/ProductController.cs:161-173) queries for a category by name and creates a new one if not found. The DbContext configuration at src/Services/Product/Product.Infrastructure/Data/ProductDbContext.cs:20-25 does not define a unique index on ProductCategory.Name. Under concurrent requests creating products with the same new category name, both requests will find no existing category and each will insert a duplicate. Over time this can lead to multiple categories with identical names in the database, with products silently split across them. A unique index on Name plus retry-on-conflict logic would prevent this.

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

Valid edge case, but intentionally out of scope here. The carve-out spec requires the fluent config to be ported verbatim from the monolith's ApplicationDbContext, which does not define a unique index on ProductCategory.Name — so adding one would diverge from source-of-truth parity. The find-or-create in ResolveCategoryAsync is a convenience for write paths (the monolith DTO is read-oriented and flattens the category name); hardening it against concurrent duplicate-name inserts (unique index + retry-on-conflict) is a reasonable follow-up but would be a deliberate deviation from the ported schema. Flagging for the requester rather than changing it in this PR.

…ed category

devin-ai-integration

Devin Review found 3 new potential issues.

View 2 additional findings in Devin Review.

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

+using (var scope = app.Services.CreateScope())
+{
+    var db = scope.ServiceProvider.GetRequiredService<ProductDbContext>();
+    var logger = scope.ServiceProvider.GetRequiredService<ILogger<Program>>();
+
+    for (var attempt = 1; attempt <= 10; attempt++)
+    {
+        try
+        {
+            db.Database.EnsureCreated();
+            break;
+        }
+        catch (Exception ex) when (attempt < 10)
+        {
+            logger.LogWarning(ex, "Database not ready (attempt {Attempt}/10); retrying in 3s...", attempt);
+            Thread.Sleep(TimeSpan.FromSeconds(3));
+        }
+    }
+}


🚩 Database initialization blocks startup — health checks unavailable during retry window

The EnsureCreated() retry loop at Program.cs:71-89 runs synchronously (using Thread.Sleep) after the middleware pipeline is built but before app.Run(). This means the HTTP server is not listening during the up-to-30-second retry window. Container orchestrators or the gateway's health probes hitting /healthz will get connection refused during this period. The smoke test at e2e-smoke.sh:19 accommodates this with a 90-second wait, but in Kubernetes with tighter liveness probe deadlines this could cause unnecessary restarts. Moving DB initialization to an IHostedService or IStartupFilter (or using await db.Database.EnsureCreatedAsync() with Task.Delay) would allow the server to start accepting health checks immediately.

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

This is the conventional EF Core startup pattern (run EnsureCreated/Migrate before app.Run()), and it's bounded: the loop only sleeps while Postgres is unreachable and breaks immediately once it's ready (typically the first attempt). The retry exists purely to absorb compose startup ordering. The validation gate confirms /healthz returns 200 once up, and the e2e script's wait accommodates the brief window. A K8s deployment would normally pair this with a startupProbe/initialDelaySeconds rather than relying on liveness during init. Moving to an IHostedService/IStartupFilter is a reasonable hardening but out of scope for this carve-out; leaving as-is.

… default Uncategorized)

devin-ai-integration

Devin Review found 3 new potential issues.

View 3 additional findings in Devin Review.

devin-ai-integration · 2026-06-26T05:03:37Z

+        var resolvedCategory = await ResolveCategoryAsync(model.ProductCategoryName);
+        if (resolvedCategory.Name != product.ProductCategory?.Name)
+        {
+            product.ProductCategory = resolvedCategory;
+        }


📝 Info: Update with null category name silently reclassifies product to 'Uncategorized'

In the Update action (ProductController.cs:95-128), ResolveCategoryAsync is always called with model.ProductCategoryName. If the API consumer sends a PUT with ProductCategoryName omitted (null), the method defaults to "Uncategorized" (ProductController.cs:170). This follows PUT semantics (full replacement), but it means a consumer intending to update only the product name must also echo back the current category name, or the product will be silently re-categorized. This is consistent with the Create behavior but could surprise API consumers expecting partial-update semantics.

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

Intentional and, as noted, correct PUT full-replacement semantics — now consistent with Create (this is exactly the consistency that the previous review round asked for). A safer partial-update story (PATCH, or making category-name required on update) is a reasonable product decision but a behavioral/API-surface change beyond this carve-out's scope, so I'm leaving the PUT semantics as-is and flagging it for the requester.

…ve Location on Create

devin-ai-integration

Devin Review found 1 new potential issue.

View 2 additional findings in Devin Review.

devin-ai-integration · 2026-06-26T05:15:20Z

+        modelBuilder.Entity<ProductCategory>(entity =>
+        {
+            entity.Property(p => p.Name).IsRequired().HasMaxLength(100);
+            entity.Property(p => p.Description).HasMaxLength(500);
+            entity.ToTable("AppProductCategories");
+        });


🚩 Cascade delete on Product→ProductCategory relationship is unconfigured

The OnModelCreating explicitly configures DeleteBehavior.Restrict for the Parent→Children self-referencing relationship (ProductDbContext.cs:35-37), but the Product→ProductCategory relationship is left to EF Core convention. Since ProductCategoryId is a non-nullable int (Product.cs:17), EF Core treats it as a required relationship and defaults to DeleteBehavior.Cascade. This means deleting a ProductCategory row would cascade-delete all its products. There is no controller endpoint to delete categories, so this isn't immediately exploitable, but if one is added later or a manual DB operation is performed, products could be silently deleted. This contrasts with the explicit Restrict on Parent→Children, suggesting the author intended defensive FK behavior.

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

This matches the source-of-truth verbatim. In the monolith's ApplicationDbContext, the only explicit OnDelete on the Product aggregate is Parent → Children (Restrict); the Product → ProductCategory relationship is left to EF convention (Cascade for the required FK) there too. The carve-out spec requires porting the fluent config verbatim, so I've preserved exactly that — adding an explicit Restrict/SetNull here would diverge from the monolith schema. As you note, there's no category-delete endpoint, so it's not reachable today. Reasonable hardening for a follow-up (alongside the category unique-index point), but I'm keeping parity in this PR and flagging it for the requester.

mbatchelor81 self-assigned this Jun 26, 2026

devin-ai-integration Bot commented Jun 26, 2026

View reviewed changes

Use null-conditional category access in Update to avoid 500 on orphan…

f2b9b9b

…ed category

devin-ai-integration Bot commented Jun 26, 2026

View reviewed changes

Make Update category handling consistent with Create (always resolve,…

b42ecdb

… default Uncategorized)

devin-ai-integration Bot commented Jun 26, 2026

View reviewed changes

Harden Delete (409 on products with children) and emit gateway-relati…

a8724fb

…ve Location on Create

devin-ai-integration Bot commented Jun 26, 2026

View reviewed changes

		[HttpGet("/")]
		[ProducesResponseType(typeof(IEnumerable<ProductVM>), StatusCodes.Status200OK)]

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

Decoupling

Domain (Product.Domain)

Persistence (Product.Infrastructure.ProductDbContext, Npgsql)

API (Product.API)

Scaffold bug fix

Validation gate (all passing)

Scope

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

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 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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

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

edited

Loading

Domain (`Product.Domain`)

Persistence (`Product.Infrastructure.ProductDbContext`, Npgsql)

API (`Product.API`)

devin-ai-integration Bot Jun 26, 2026 •

edited

Loading