Files

Ho Ngoc Hai 36a0a9c256 feat: Add functional tests for MktZaloService, new contract and load tests, and audit documentation, while removing a legacy infrastructure project and updating service configurations.

2026-03-25 15:00:05 +07:00

15 KiB

Raw Blame History

Database Architecture Audit — GoodGo POS System

Auditor: Database Architect (TechBi Company) Date: 2026-03-20 Scope: Schema design, migrations, indexing, query performance, data modeling Coverage: 23 microservices, 24 PostgreSQL databases, Redis 7 caching layer

Executive Summary

The platform follows a sound database-per-service architecture with EF Core 10 + Fluent API configuration, domain-driven entity design, and consistent snake_case column naming across most services. The foundation is solid for a POS microservices platform. However, three critical issues require immediate attention: a broken InventoryItem.CreatedAt field mapping that silently returns DateTime.MinValue from the database, missing optimistic concurrency on wallet balance updates (race condition risk under concurrent POS load), and UUID v4 used universally instead of the UUID v7 standard specified in the architecture guidelines. Additionally, IAM's Phase 4A migration introduced PascalCase table names that break the snake_case convention across the rest of the system.

Critical Issues

C-1 · `InventoryItem.CreatedAt` Backing Field Not Mapped — Silent Data Loss

File: services/inventory-service-net/src/InventoryService.Infrastructure/EntityConfigurations/InventoryItemEntityTypeConfiguration.cs:34

// WRONG — Ignores public property but NEVER maps the private backing field _createdAt
builder.Ignore(i => i.CreatedAt);

The domain entity has private DateTime _createdAt and public DateTime CreatedAt => _createdAt;. The created_at column exists in the migration and database. However, because builder.Ignore(i => i.CreatedAt) discards the property and no builder.Property<DateTime>("_createdAt").HasColumnName("created_at") mapping exists, EF Core never reads created_at from the DB into the backing field. Every entity loaded from the database will have CreatedAt == DateTime.MinValue.

Compare with the correct pattern used in InventoryItemEntityTypeConfiguration for other fields:

builder.Property(i => i.ShopId).HasField("_shopId").HasColumnName("shop_id").IsRequired(); // ✓ correct

Fix:

// Replace builder.Ignore(i => i.CreatedAt) with:
builder.Property<DateTime>("_createdAt")
    .HasColumnName("created_at")
    .IsRequired();
builder.Ignore(i => i.CreatedAt); // keep ignoring the computed public property

No migration needed — the column already exists.

C-2 · No Optimistic Concurrency on Wallet Balances — Race Condition Risk

File: services/wallet-service-net/src/WalletService.Infrastructure/EntityConfigurations/WalletItemEntityTypeConfiguration.cs

wallet_items.balance (the per-currency balance) has no concurrency token. In a busy POS terminal, two concurrent Debit or Credit operations on the same wallet can interleave:

Tx1: READ balance=100 → compute 100-30=70
Tx2: READ balance=100 → compute 100-50=50
Tx1: WRITE balance=70
Tx2: WRITE balance=50  ← overwrites Tx1's debit, 30 VND disappears

Fix — add PostgreSQL xmin system column as concurrency token:

// WalletItemEntityTypeConfiguration.cs
builder.UseXminAsConcurrencyToken(); // Npgsql built-in

No migration needed — xmin is a PostgreSQL system column. EF Core will throw DbUpdateConcurrencyException on conflict, which the application must retry.

Also applies to wallet_transactions if idempotent writes are not already enforced by ClientRequest idempotency table.

C-3 · UUID v4 (`Guid.NewGuid()`) Used Everywhere Instead of UUID v7

Files: All domain aggregate roots across all 23 services

Architecture spec mandates UUID v7 (time-sortable, B-tree friendly). Every service uses Guid.NewGuid() (UUID v4 — random). At scale, random UUIDs cause index fragmentation in PostgreSQL B-tree indexes: each new row's primary key is a random value, forcing page splits and bloating indexes.

Examples:

services/order-service-net/src/OrderService.Domain/AggregatesModel/OrderAggregate/Order.cs:163 — Id = Guid.NewGuid()
services/merchant-service-net/src/MerchantService.Domain/AggregatesModel/ShopAggregate/Shop.cs:191 — Id = Guid.NewGuid()

Fix (.NET 9+):

// Create a shared utility or use directly:
Id = Guid.CreateVersion7(); // .NET 9+ built-in, time-sortable

This is a cross-cutting change. Recommend introducing a NewId() factory in the SeedWork/Entity.cs base class:

protected static Guid NewId() => Guid.CreateVersion7();

C-4 · IAM Phase4A Migration — PascalCase Table Names Break Convention

File: services/iam-service-net/src/IamService.Infrastructure/Migrations/20260114091114_Phase4A_AuditAndCompliance.cs

Audit and compliance tables were created with PascalCase names, breaking the snake_case convention used by every other service:

Table Created	Expected
`AuditLogs`	`audit_logs`
`ComplianceReports`	`compliance_reports`
`ComplianceViolations`	`compliance_violations`
`IX_AuditLogs_ActorId`	`ix_audit_logs_actor_id`
`IX_AuditLogs_Timestamp`	`ix_audit_logs_timestamp`

PostgreSQL is case-insensitive for unquoted identifiers but stores them lowercase. Quoted identifiers in EF queries will fail if mixed. The AuditLogRepository likely works by accident (EF generates quoted DDL), but it creates a maintainability hazard.

Fix — additive migration:

ALTER TABLE "AuditLogs" RENAME TO audit_logs;
ALTER TABLE "ComplianceReports" RENAME TO compliance_reports;
ALTER TABLE "ComplianceViolations" RENAME TO compliance_violations;
ALTER INDEX "IX_AuditLogs_ActorId" RENAME TO ix_audit_logs_actor_id;
-- ... etc

Then update EntityTypeConfiguration and AuditLogEntityTypeConfiguration to use builder.ToTable("audit_logs").

Also note: OpenIddict tables (OpenIddictApplications, OpenIddictTokens, etc.) in the same IAM database use PascalCase — this is OpenIddict's default and acceptable since it manages its own schema, but is worth documenting.

Warnings

W-1 · Missing Global Query Filter for Soft-Deleted Records

File: services/merchant-service-net/src/MerchantService.Infrastructure/MerchantServiceContext.cs

Both Merchant and Shop entities have is_deleted columns (correctly designed). However, MerchantServiceContext.OnModelCreating does not register a global query filter:

// Missing in MerchantServiceContext.OnModelCreating:
modelBuilder.Entity<Merchant>()
    .HasQueryFilter(m => EF.Property<bool>(m, "_isDeleted") == false);
modelBuilder.Entity<Shop>()
    .HasQueryFilter(s => EF.Property<bool>(s, "_isDeleted") == false);

Without this, every repository query must manually add .Where(x => !x.IsDeleted). Any query that omits this filter will return deleted records silently. This is a data leak risk.

Fix: Add HasQueryFilter in MerchantServiceContext.OnModelCreating. Ensure IgnoreQueryFilters() is available for admin/hard-delete operations.

W-2 · Missing Composite Indexes on High-Traffic Query Patterns

The most common POS reporting queries are not supported by composite indexes:

Service	Missing Index	Common Query
`order-service-net`	`(shop_id, created_at DESC)`	"Orders today for shop X"
`order-service-net`	`(shop_id, status_id)`	"Pending orders for shop X"
`catalog-service-net`	`(shop_id, is_active)`	"Active products for shop X"
`booking-service-net`	`(shop_id, start_time)`	"Appointments today for shop X"
`inventory-service-net`	`(shop_id, quantity)`	"Low stock items for shop X"

Current separate single-column indexes (ix_orders_shop_id + ix_orders_created_at) require a bitmap index scan merge, which is slower than a single composite index for the WHERE shop_id = ? AND created_at BETWEEN ? AND ? pattern.

Fix example for orders:

// OrderEntityTypeConfiguration.cs
builder.HasIndex("_shopId", "_createdAt")
    .HasDatabaseName("ix_orders_shop_id_created_at");
builder.HasIndex("_shopId", "StatusId")
    .HasDatabaseName("ix_orders_shop_id_status_id");

W-3 · Index Naming Inconsistency — `idx_` vs `ix_` Prefix

Files: services/ads-analytics-service-net/src/AdsAnalyticsService.Infrastructure/EntityConfigurations/

All services use ix_ prefix for index names. The AdsAnalyticsService (and its ReportEntityTypeConfiguration) uses idx_ prefix:

idx_campaign_metrics_campaign_id ← should be ix_campaign_metrics_campaign_id
idx_reports_advertiser_id ← should be ix_reports_advertiser_id

Minor but affects tooling, migrations consistency, and DBA runbooks.

W-4 · `Debug.WriteLine` Left in Production Code

File: services/order-service-net/src/OrderService.Infrastructure/OrderContext.cs:59,71

System.Diagnostics.Debug.WriteLine("OrderContext::ctor - " + GetHashCode());
System.Diagnostics.Debug.WriteLine("OrderContext::ctor (tenant) - " + GetHashCode());

Debug output left in a production DbContext constructor. Under load, this generates noise. Should be removed or replaced with structured ILogger at Debug level.

W-5 · Inconsistent DbContext File Naming

File: services/mkt-facebook-service-net/src/FacebookService.Infrastructure/MyServiceContext.cs

The file is named MyServiceContext.cs — the template placeholder name was not updated when the service was created. The class inside is correctly named FacebookServiceContext, but the file name reveals copy-paste from _template_dot_net without cleanup. Affects code navigation and grep tooling.

W-6 · `ads-analytics-service-net` and `ads-billing-service-net` Have No Repositories

These two services access DbContext directly from command/query handlers, bypassing the Repository pattern used everywhere else. This:

Makes unit testing harder (must mock DbContext instead of a simple IRepository)
Breaks the Clean Architecture boundary (Infrastructure DbContext leaks into Application layer)
Creates inconsistency for future maintainers

W-7 · Missing `expires_at` Index on Vouchers

File: services/promotion-service-net/src/PromotionService.Infrastructure/EntityConfigurations/VoucherEntityTypeConfiguration.cs

Voucher expiry queries (WHERE expires_at < NOW() AND status_id = 'active') have no index on expires_at. For a promotion system that may hold millions of vouchers, periodic expiry sweeps will cause sequential scans.

// Add:
builder.HasIndex(v => v.ExpiresAt)
    .HasDatabaseName("ix_vouchers_expires_at")
    .HasFilter("expires_at IS NOT NULL");

Improvements

I-1 · Enable `pg_stat_statements` in `init.sql`

File: infra/databases/postgres/init.sql

The init.sql is nearly empty. Add the essential PostgreSQL extension for query monitoring:

CREATE EXTENSION IF NOT EXISTS "pg_stat_statements";
CREATE EXTENSION IF NOT EXISTS "uuid-ossp"; -- useful for uuid_generate_v7() if Guid.CreateVersion7 is not used

I-2 · Add PgBouncer for Local Development

File: deployments/local/docker-compose.yml

PgBouncer is referenced in staging/production (via Neon's built-in pooler with ?pgbouncer=true). However, local development has no connection pooler. With 25+ microservices each holding a connection pool, the local PostgreSQL can hit max_connections=200 quickly during integration testing.

Add a lightweight PgBouncer container to docker-compose.yml:

pgbouncer:
  image: bitnami/pgbouncer:1.22
  environment:
    POSTGRESQL_HOST: postgres
    POSTGRESQL_PORT: 5432
    PGBOUNCER_POOL_MODE: transaction
    PGBOUNCER_MAX_CLIENT_CONN: 500
    PGBOUNCER_DEFAULT_POOL_SIZE: 10

Services should connect to pgbouncer:5432 locally.

I-3 · Consider Table Partitioning for High-Volume Append-Only Tables

As the platform grows, these tables will accumulate millions of rows and benefit from range partitioning by month:

Table	Service	Strategy
`wallet_transactions`	wallet-service	RANGE on `created_at`
`inventory_transactions`	inventory-service	RANGE on `created_at`
`order_items`	order-service	inherited from `orders` RANGE on `created_at`
`audit_logs` (IAM)	iam-service	RANGE on `timestamp`

EF Core 10 + Npgsql support declarative partitioning. This is not urgent for current scale but should be planned for tables expected to exceed 10M rows.

I-4 · Add Partial Index for Active Products

File: catalog-service-net

For POS menu loading, ~90% of queries filter on is_active = true. A partial index reduces index size by ~10x:

builder.HasIndex(p => new { p.ShopId, p.CategoryId })
    .HasDatabaseName("ix_products_active_shop_category")
    .HasFilter("is_active = true");

I-5 · Materialized View for Daily Sales Reporting

Service: order-service-net

The most common merchant dashboard query — "sales by day for the last 30 days" — requires aggregating orders and order_items over large date ranges. A nightly-refreshed materialized view would eliminate expensive aggregation queries at runtime:

CREATE MATERIALIZED VIEW mv_daily_sales AS
SELECT
    shop_id,
    DATE(created_at) AS sale_date,
    COUNT(*) AS order_count,
    SUM(total_amount) AS total_revenue,
    SUM(discount_amount) AS total_discount
FROM orders
WHERE status_id != 4 -- exclude cancelled
GROUP BY shop_id, DATE(created_at);

CREATE UNIQUE INDEX ON mv_daily_sales(shop_id, sale_date);

Refresh with REFRESH MATERIALIZED VIEW CONCURRENTLY mv_daily_sales; via a scheduled job.

Action Items (Prioritized)

Priority	Item	Service(s)	Effort
P0	Fix `InventoryItem._createdAt` backing field mapping	`inventory-service-net`	30 min
P0	Add optimistic concurrency token to `wallet_items`	`wallet-service-net`	1 hour
P1	Migrate IAM AuditLogs/ComplianceReports to snake_case	`iam-service-net`	2 hours
P1	Add `HasQueryFilter` for soft-deleted Merchant + Shop	`merchant-service-net`	1 hour
P1	Replace `Guid.NewGuid()` with `Guid.CreateVersion7()` in all domain entities	All 23 services	4 hours
P2	Add composite indexes: `(shop_id, created_at)` for orders, `(shop_id, is_active)` for products	order, catalog	1 hour
P2	Remove `Debug.WriteLine` from `OrderContext` constructors	`order-service-net`	10 min
P2	Add `expires_at` index on `vouchers`	`promotion-service-net`	15 min
P2	Standardize index prefix to `ix_` in ads-analytics	`ads-analytics-service-net`	30 min
P3	Rename `MyServiceContext.cs` to `FacebookServiceContext.cs`	`mkt-facebook-service-net`	5 min
P3	Add PgBouncer to local docker-compose	`deployments/local`	2 hours
P3	Enable `pg_stat_statements` in `init.sql`	`infra/databases`	15 min
P3	Add Repository pattern to ads-analytics + ads-billing	`ads-*-service-net`	4 hours

Database Inventory Summary

Category	Count
Microservices with own database	23
Total migrations	46
DbContext files	24
EntityTypeConfiguration files	95
Repository implementations	~91
PostgreSQL databases (local)	24
Redis usage	SignalR backplane (chat), caching

Overall Assessment: Architecture is well-structured and production-viable. The critical issues (C-1 to C-4) should be resolved before the next production deployment. Warnings are tech debt that should be addressed in the next sprint.

15 KiB Raw Blame History