765cc632d5
Remove the Order demo (entity/feature/repo/config/gRPC/proto) and the three pre-marketplace migrations; regenerate a fresh InitialBaseline migration. Stand up the REST surface (PingController + System/Ping CQRS) proving the Mediator -> behaviors -> OperationResult -> ApiResult envelope end to end. Close wiring gaps: register LoggingBehavior (outermost) and add the built-in rate limiter (per-IP global + otp/auth/sensitive policies), placed before authentication. Add current-user + audit plumbing: ICurrentUser (HttpContext + null impls), rename BaseEntity audit fields to CreatedAt/ModifiedAt (DateTimeOffset) + CreatedById/ModifiedById, stamped by a new AuditFieldInterceptor. Introduce five cross-cutting seams (IDateTimeProvider, IFieldEncryptor, ICacheService, IObjectStorage, INotificationDispatcher) with in-memory/local mocks registered via AddCrossCuttingSeams. Add Baya.Test.Foundation (encryptor, audit interceptor, ping handler) and update docs, contracts (swagger.v1.json), handoff, report, and mocks registry.
433 lines
19 KiB
Markdown
433 lines
19 KiB
Markdown
# Server Coding Conventions
|
|
|
|
Rules enforced for all code in `server/`. These represent the standards expected from a **senior .NET engineer**. Read alongside [CLAUDE.md](CLAUDE.md).
|
|
|
|
When in doubt, ask: _would a senior engineer approve this diff without comment?_
|
|
|
|
---
|
|
|
|
## 1. Routing
|
|
|
|
### Rule: all URL segments must be `snake_case`
|
|
|
|
`SnakeCaseParameterTransformer` (`Baya.WebFramework/Routing/`) is registered globally via `RouteTokenTransformerConvention`. It converts `[controller]` and `[action]` tokens automatically.
|
|
|
|
```csharp
|
|
// ✅ transformer converts MyFeature → my_feature, GetBySlug → get_by_slug
|
|
[Route("api/v{version:apiVersion}/[controller]")]
|
|
public class MyFeatureController : BaseController
|
|
{
|
|
[HttpGet("[action]")]
|
|
public Task<IActionResult> GetBySlug(...) { }
|
|
}
|
|
|
|
// ❌ bypasses transformer — hardcoded segment escapes snake_case enforcement
|
|
[Route("api/v{version:apiVersion}/MyFeature")]
|
|
[HttpGet("GetBySlug")]
|
|
```
|
|
|
|
If a method name doesn't read cleanly as a URL, **rename the method** — don't hardcode the route string.
|
|
|
|
---
|
|
|
|
## 2. C# code quality
|
|
|
|
### Use the right type for the job
|
|
|
|
| Scenario | Use |
|
|
|---|---|
|
|
| Request/response/DTO | `record` (immutable, value semantics) |
|
|
| Domain entity | `class` (mutable state, encapsulated) |
|
|
| Shared small value | `readonly record struct` |
|
|
| Handler, service | `sealed class` |
|
|
|
|
### Language features — use them
|
|
|
|
```csharp
|
|
// ✅ primary constructor (C# 12)
|
|
public sealed class OrderHandler(IUnitOfWork uow, IMapper mapper) : IRequestHandler<...> { }
|
|
|
|
// ✅ switch expression over if/else chains
|
|
var label = status switch
|
|
{
|
|
OrderStatus.Pending => "Pending",
|
|
OrderStatus.Shipped => "Shipped",
|
|
OrderStatus.Cancelled => "Cancelled",
|
|
_ => throw new ArgumentOutOfRangeException(nameof(status))
|
|
};
|
|
|
|
// ✅ pattern matching
|
|
if (result is { IsSuccess: false, IsNotFound: true }) return NotFound();
|
|
|
|
// ✅ collection expressions (C# 12)
|
|
List<string> tags = ["new", "sale"];
|
|
```
|
|
|
|
### Immutability & safety
|
|
|
|
- Mark fields `readonly` unless mutation is genuinely needed.
|
|
- Prefer `IReadOnlyList<T>` / `IReadOnlyCollection<T>` over `List<T>` in signatures unless the caller needs to mutate.
|
|
- Never expose public setters on entities — use methods or constructors.
|
|
- Avoid `static` mutable state.
|
|
|
|
### Null handling
|
|
|
|
- Enable `<Nullable>enable</Nullable>` in any new project you create.
|
|
- Use guard clauses at the entry point; don't scatter null checks throughout.
|
|
- Prefer returning `OperationResult.NotFoundResult(...)` over returning `null` from handlers.
|
|
- Never use `null!` (null-forgiving) unless you can prove the value cannot be null and the compiler cannot.
|
|
|
|
### Naming
|
|
|
|
| Kind | Convention | Example |
|
|
|---|---|---|
|
|
| Class, record, interface | PascalCase | `OrderHandler`, `IOrderRepository` |
|
|
| Method | PascalCase | `GetUserOrdersAsync` |
|
|
| Parameter, local variable | camelCase | `orderId`, `userEmail` |
|
|
| Private field | `_camelCase` | `_unitOfWork` |
|
|
| Constant | PascalCase | `MaxRetryCount` |
|
|
| Generic type param | `T` or descriptive `TEntity` | |
|
|
| Command | `{Verb}{Noun}Command` | `CreateOrderCommand` |
|
|
| Query | `{Verb}{Noun}Query` | `GetUserOrdersQuery` |
|
|
| Handler | `{RequestName}Handler` | `CreateOrderCommandHandler` |
|
|
| Result DTO | `{RequestName}Result` | `CreateOrderCommandResult` |
|
|
|
|
No abbreviations unless universally understood (`dto`, `id`, `url`). No Hungarian notation (`strName`, `intCount`).
|
|
|
|
### No unused code
|
|
|
|
Leave nothing dead behind. Remove unused `using` directives, local variables, parameters, private fields, and private members rather than letting them accumulate.
|
|
|
|
- These already surface as compiler/analyzer signals — `CS0168` (variable declared, never used), `CS0219` (variable assigned, value never used), `CS0169` (private field never used), `IDE0005` (unnecessary `using`). The quality gate is **zero new warnings**, so treat unused code as a gate failure.
|
|
- **Delete it — don't silence it.** Do not add `#pragma warning disable`, throwaway discards, or `_ =` assignments just to quiet the analyzer.
|
|
- The one exception: a parameter that must exist to satisfy an interface or delegate signature but is genuinely unused. Keep it, name it conventionally, and add a one-line `// why` only if the reason isn't obvious.
|
|
|
|
### Comments — explain *why*, never *what*
|
|
|
|
Code that needs a comment to be understood usually needs a better name instead. Prefer self-documenting names over prose.
|
|
|
|
- **Do not** write comments that restate what the code already says — no `// constructor`, `// loop over users`, or XML-doc that merely echoes the method name.
|
|
- **Do** add a comment only where a non-obvious decision, constraint, business rule, workaround, or trade-off is *not* evident from the code — explain the reasoning, not the mechanics.
|
|
- Keep any necessary comment tight, and delete comments that no longer match the code.
|
|
|
|
```csharp
|
|
// ❌ restates the obvious
|
|
// increment the retry counter
|
|
retryCount++;
|
|
|
|
// ✅ captures a non-obvious constraint the code can't express on its own
|
|
// Payment gateway rejects amounts above 50M IRR per call; split larger settlements upstream.
|
|
if (amount > MaxPerCallRial) ...
|
|
```
|
|
|
|
---
|
|
|
|
## 3. Async / await
|
|
|
|
```csharp
|
|
// ✅ always async all the way — no .Result or .Wait()
|
|
public async ValueTask<OperationResult<T>> Handle(MyQuery request, CancellationToken ct)
|
|
{
|
|
var entity = await _repository.GetAsync(request.Id, ct);
|
|
return OperationResult<T>.SuccessResult(_mapper.Map(entity));
|
|
}
|
|
|
|
// ❌ blocks the thread, risks deadlock
|
|
var result = _repository.GetAsync(id).Result;
|
|
|
|
// ✅ pass CancellationToken through every async call
|
|
await _db.SaveChangesAsync(cancellationToken);
|
|
|
|
// ❌ fire and forget with no error handling
|
|
_ = DoSomethingAsync();
|
|
```
|
|
|
|
- Every public async method must accept `CancellationToken` and pass it downstream.
|
|
- Use `ValueTask<T>` for hot paths (handlers, repositories). Use `Task<T>` for rarely-called or always-async methods.
|
|
- Never use `async void` — it swallows exceptions. Use `async Task` even for event-like callbacks.
|
|
- Do not add `.ConfigureAwait(false)` in this ASP.NET Core app — it's unnecessary and adds noise.
|
|
|
|
---
|
|
|
|
## 4. Controllers
|
|
|
|
Every controller must follow this skeleton:
|
|
|
|
```csharp
|
|
[ApiVersion("1")]
|
|
[ApiController]
|
|
[Route("api/v{version:apiVersion}/[controller]")]
|
|
[Display(Description = "One-line description shown in Swagger")]
|
|
[Authorize(ConstantPolicies.DynamicPermission)] // or [Authorize], or omit for public
|
|
public sealed class MyFeatureController(ISender sender) : BaseController
|
|
{
|
|
[HttpGet("[action]")]
|
|
[ProducesOkApiResponseType<MyQueryResult>]
|
|
public async Task<IActionResult> GetSomething(CancellationToken ct)
|
|
=> OperationResult(await sender.Send(new MyQuery(), ct));
|
|
|
|
[HttpPost("[action]")]
|
|
[ProducesOkApiResponseType<MyCommandResult>]
|
|
public async Task<IActionResult> CreateSomething(MyCommand command, CancellationToken ct)
|
|
=> OperationResult(await sender.Send(command, ct));
|
|
}
|
|
```
|
|
|
|
Rules:
|
|
- `sealed` — controllers are not designed for inheritance beyond `BaseController`.
|
|
- Inject `ISender` via primary constructor — not `IMediator`.
|
|
- **Never call `Ok()`, `BadRequest()`, `NotFound()` directly** — always `base.OperationResult(result)`.
|
|
- Keep controller methods thin: one `Send`, one `OperationResult`. No business logic in controllers.
|
|
- Use `[Display(Description = "...")]` so NSwag generates meaningful Swagger tags.
|
|
- Pass `CancellationToken` from the action into `sender.Send(...)`.
|
|
|
|
### Authorization levels — use the narrowest that fits
|
|
|
|
| Attribute | When |
|
|
|---|---|
|
|
| _(none)_ | Truly public (health check, metrics) |
|
|
| `[Authorize]` | Any authenticated user |
|
|
| `[Authorize(ConstantPolicies.DynamicPermission)]` | Role/claim-gated admin action |
|
|
| `[RequireTokenWithoutAuthorization]` | Token must be present but may be expired (e.g. refresh) |
|
|
|
|
Apply at the **controller level** for uniform policy; override at the action level only for exceptions.
|
|
|
|
---
|
|
|
|
## 5. CQRS — feature structure
|
|
|
|
```
|
|
Features/<Area>/
|
|
├── Commands/<VerbNoun>Command/
|
|
│ ├── <Name>Command.cs record Command(…) : IRequest<OperationResult<T>>
|
|
│ ├── <Name>Command.Handler.cs internal sealed class Handler : IRequestHandler<…>
|
|
│ └── <Name>Command.Validator.cs AbstractValidator<Command> (omit if no validation needed)
|
|
└── Queries/<VerbNoun>Query/
|
|
├── <Name>Query.cs record Query(…) : IRequest<OperationResult<T>>
|
|
├── <Name>Query.Handler.cs internal sealed class Handler : IRequestHandler<…>
|
|
└── <Name>Query.Result.cs record Result(…) ← the DTO returned
|
|
```
|
|
|
|
- Request types are `record` — immutable.
|
|
- Handlers are `internal sealed` — they are never used outside the Application layer.
|
|
- **Handlers must not throw for expected failures.** Use `OperationResult` factory methods:
|
|
- `OperationResult<T>.SuccessResult(value)` — happy path
|
|
- `OperationResult<T>.FailureResult(errors)` — validation / business rule failure
|
|
- `OperationResult<T>.NotFoundResult(message)` — entity not found
|
|
- Only one handler per request type — no conditional dispatch.
|
|
- Contracts the handler depends on go in `Application/Contracts/` as interfaces; implementations live in Infrastructure.
|
|
|
|
---
|
|
|
|
## 6. Persistence — EF Core rules
|
|
|
|
```csharp
|
|
// ✅ project to DTO in the query — never load full entity for read operations
|
|
var dto = await _db.Orders
|
|
.AsNoTracking()
|
|
.Where(o => o.UserId == userId)
|
|
.Select(o => new OrderResult(o.Id, o.Status, o.CreatedAt))
|
|
.ToListAsync(ct);
|
|
|
|
// ❌ loads entire entity graph then maps in memory — N+1 risk
|
|
var orders = await _db.Orders.Include(o => o.Lines).ToListAsync();
|
|
var dtos = _mapper.Map<List<OrderResult>>(orders);
|
|
```
|
|
|
|
Rules:
|
|
- **Always use `AsNoTracking()`** on read-only queries.
|
|
- **Always project with `Select()`** in queries — never hydrate full entities just to map them.
|
|
- Never load more than you need. Pagination is mandatory for any unbounded list: `Skip` / `Take`.
|
|
- Use `Include` only in command handlers where you need to mutate the aggregate and need navigation properties loaded.
|
|
- Access the DB through `IUnitOfWork` in Application-layer handlers. `ApplicationDbContext` is only referenced directly inside Infrastructure.
|
|
- Commit once per command at the end: `await _unitOfWork.CommitAsync(ct)`.
|
|
- One `IEntityTypeConfiguration<T>` per entity, in `Persistence/Configuration/<Area>Config/`.
|
|
- Migrations command: `dotnet ef migrations add <Name> --project src/Infrastructure/Baya.Infrastructure.Persistence --startup-project src/API/Baya.Web.Api`
|
|
|
|
### Soft delete
|
|
|
|
Every entity that supports soft delete **must** declare a global EF query filter in its `IEntityTypeConfiguration<T>`:
|
|
|
|
```csharp
|
|
public void Configure(EntityTypeBuilder<Order> builder)
|
|
{
|
|
builder.HasQueryFilter(o => !o.IsDeleted);
|
|
}
|
|
```
|
|
|
|
Without this filter, soft-deleted records appear in every query that doesn't explicitly filter them — a silent data leak. Never add `Where(x => !x.IsDeleted)` in individual queries; the filter makes it automatic and auditable.
|
|
|
|
### Entity audit fields
|
|
|
|
When designing or extending an entity, include audit fields alongside timestamps:
|
|
|
|
| Field | Type | Set by |
|
|
|---|---|---|
|
|
| `CreatedAt` | `DateTimeOffset` | `SaveChangesAsync` override (on Add) |
|
|
| `ModifiedAt` | `DateTimeOffset` | `SaveChangesAsync` override (on Update) |
|
|
| `CreatedById` | `int?` | `SaveChangesAsync` override via `ICurrentUser` |
|
|
| `ModifiedById` | `int?` | `SaveChangesAsync` override via `ICurrentUser` |
|
|
|
|
Wire `ICurrentUser` (HTTP context accessor wrapped in an interface, registered Scoped) into `ApplicationDbContext` so the context can stamp who made the change without handlers needing to pass it explicitly. Audit fields cannot be backfilled retroactively — design them in from the start.
|
|
|
|
> **As built (backend-phase-0):** the audit base type is `BaseEntity`/`IAuditableEntity` in
|
|
> `Baya.Domain/Common/BaseEntity.cs` (`CreatedAt`/`ModifiedAt` as `DateTimeOffset`, `CreatedById`/
|
|
> `ModifiedById` as `int?`). Stamping is done by `AuditFieldInterceptor`
|
|
> (`Baya.Infrastructure.Persistence/Interceptors/`), a `SaveChangesInterceptor` that reads time from
|
|
> `IDateTimeProvider` and the user from `ICurrentUser` — not in the `DbContext` itself.
|
|
|
|
### Money is IRR `BIGINT` — integer-only, no floats
|
|
|
|
Every monetary value is **IRR Rials stored as `long` / `BIGINT`**. There is **no float/decimal path** on money — not in entities, DTOs, the API, or arithmetic. Toman is display-only and converts to/from Rials **only** inside a provider adapter at its boundary, never in domain or shared code. If a money value object is introduced later it must be integer-only. The three booking amounts always satisfy `gross = commission + payout`.
|
|
|
|
---
|
|
|
|
## 7. Validation
|
|
|
|
- All commands that accept user input need a `FluentValidation` validator. The `ValidateCommandBehavior` pipeline behavior runs it automatically before the handler.
|
|
- Validators are registered automatically via `RegisterValidatorsAsServices()` in `Program.cs`.
|
|
- Validate at the boundary (command/query), not deep in the domain or repositories.
|
|
|
|
```csharp
|
|
public sealed class CreateOrderCommandValidator : AbstractValidator<CreateOrderCommand>
|
|
{
|
|
public CreateOrderCommandValidator()
|
|
{
|
|
RuleFor(x => x.UserId).GreaterThan(0);
|
|
RuleFor(x => x.Items).NotEmpty().WithMessage("Order must have at least one item.");
|
|
RuleForEach(x => x.Items).ChildRules(item =>
|
|
{
|
|
item.RuleFor(i => i.ProductId).GreaterThan(0);
|
|
item.RuleFor(i => i.Quantity).InclusiveBetween(1, 100);
|
|
});
|
|
}
|
|
}
|
|
```
|
|
|
|
---
|
|
|
|
## 8. Mapping — Mapster rules
|
|
|
|
- Use `IMapper` (injected via DI) for all entity↔DTO mapping in handlers.
|
|
- Register type adapter configs in `Program.cs` via `TypeAdapterConfig.GlobalSettings.Scan(...)`. Add new assemblies that contain mapping configs there.
|
|
- Never write manual mapping code when Mapster can infer it — only write custom `TypeAdapterConfig` when shapes diverge.
|
|
- Mapping happens **in the handler after the DB query**, not in the repository.
|
|
|
|
---
|
|
|
|
## 9. Error handling & logging
|
|
|
|
```csharp
|
|
// ✅ expected failure — use OperationResult, do not throw
|
|
if (user is null)
|
|
return OperationResult<T>.NotFoundResult("User not found.");
|
|
|
|
// ✅ unexpected failure — let it propagate; ExceptionHandler middleware catches it
|
|
// Log at the point you catch unexpected exceptions (ExceptionHandler logs automatically)
|
|
|
|
// ❌ swallowing exceptions
|
|
try { ... } catch { return OperationResult<T>.FailureResult(...); }
|
|
|
|
// ✅ structured logging — never interpolate sensitive data
|
|
_logger.LogInformation("Order {OrderId} created for user {UserId}", order.Id, userId);
|
|
|
|
// ❌ logs PII / secrets
|
|
_logger.LogInformation($"Token for {user.Email}: {token}");
|
|
```
|
|
|
|
- Log at the correct level: `Debug` for trace info, `Information` for meaningful events, `Warning` for recoverable issues, `Error` for unexpected failures.
|
|
- Never log passwords, tokens, secrets, or full PII (email is borderline — use `userId` in logs instead).
|
|
- The global `ExceptionHandler` middleware catches unhandled exceptions — do not add try/catch in handlers for unknown exceptions; let them propagate.
|
|
|
|
---
|
|
|
|
## 10. Testing
|
|
|
|
### Arrange — Act — Assert, always
|
|
|
|
```csharp
|
|
[Fact]
|
|
public async Task CreateOrder_ValidCommand_ReturnsSuccess()
|
|
{
|
|
// Arrange
|
|
var command = new CreateOrderCommand(UserId: 1, Items: [new(ProductId: 5, Quantity: 2)]);
|
|
var handler = new CreateOrderCommandHandler(_unitOfWork, _mapper);
|
|
|
|
// Act
|
|
var result = await handler.Handle(command, CancellationToken.None);
|
|
|
|
// Assert
|
|
result.IsSuccess.Should().BeTrue();
|
|
result.Result.Should().NotBeNull();
|
|
}
|
|
```
|
|
|
|
- Test the **handler directly** — not the controller. Controllers are thin wrappers.
|
|
- Use `NSubstitute` for mocking: `Substitute.For<IUnitOfWork>()`.
|
|
- Integration tests use `Baya.Tests.Setup` which provides an in-memory SQLite context — prefer this over mocking the DB for persistence tests.
|
|
- Name tests: `{MethodUnderTest}_{Scenario}_{ExpectedOutcome}`.
|
|
- One assertion concept per test. Multiple `.Should()` calls are fine if they all verify the same outcome.
|
|
- Do not test EF internals (entity tracking, migrations) — test behavior through the handler.
|
|
|
|
### Integration tests — HTTP pipeline coverage
|
|
|
|
Handler tests verify business logic but leave the entire HTTP stack (routing, auth pipeline, middleware, `OperationResult → IActionResult` translation) untested. Each feature area must have at least one `WebApplicationFactory<Program>`-based test covering:
|
|
|
|
1. Happy path — authenticated request returns 200 with correct body shape.
|
|
2. Unauthenticated request returns 401.
|
|
3. Validation failure returns 400 with field-level error detail.
|
|
|
|
```csharp
|
|
public class MyFeatureApiTests(WebApplicationFactory<Program> factory)
|
|
: IClassFixture<WebApplicationFactory<Program>>
|
|
{
|
|
[Fact]
|
|
public async Task GetSomething_Authenticated_Returns200()
|
|
{
|
|
var client = factory.CreateClient();
|
|
client.DefaultRequestHeaders.Authorization =
|
|
new AuthenticationHeaderValue("Bearer", TestTokens.ValidAdminToken);
|
|
|
|
var response = await client.GetAsync("/api/v1/my_feature/get_something");
|
|
|
|
response.StatusCode.Should().Be(HttpStatusCode.OK);
|
|
}
|
|
}
|
|
```
|
|
|
|
Place these tests in a dedicated `Baya.Test.Api` project so they can run against the full `Program.cs` wiring.
|
|
|
|
---
|
|
|
|
## 11. Security rules
|
|
|
|
- **Never hardcode secrets.** Keys, connection strings, and tokens come from `appsettings.*.json` / user-secrets / environment variables, bound to typed settings classes.
|
|
- `SecretKey` and `Encryptkey` (in `IdentitySettings`) must be set in environment-specific config, never in `appsettings.json` committed to the repo.
|
|
- Always validate all external input with FluentValidation before processing.
|
|
- EF Core parameterizes queries automatically — never concatenate raw SQL.
|
|
- If you must use raw SQL, use `FromSqlInterpolated` (parameterized), never `FromSqlRaw` with user data.
|
|
- Respect the principle of least privilege: grant `[Authorize(ConstantPolicies.DynamicPermission)]` to admin actions, not just `[Authorize]`.
|
|
- **Auth and OTP endpoints must be rate-limited.** Use ASP.NET Core's built-in `AddRateLimiter` (no extra NuGet package needed). Apply at minimum to: login, OTP request, and token refresh. A fixed window or token bucket policy per IP is the baseline. Register the limiter in a `ServiceConfiguration/` extension; add `app.UseRateLimiter()` before `app.UseAuthentication()` in `Program.cs`.
|
|
|
|
---
|
|
|
|
## 12. Service registration
|
|
|
|
- Every new infrastructure service gets an extension method in the project's `ServiceConfiguration/` folder.
|
|
- That extension is called from `Program.cs` — no inline DI registration in `Program.cs`.
|
|
- Register with the correct lifetime:
|
|
- **Singleton** — stateless, thread-safe services (e.g. `IHttpContextAccessor`)
|
|
- **Scoped** — per-request services (repositories, `DbContext`, handlers)
|
|
- **Transient** — lightweight, stateless (validators, transformers)
|
|
- All NuGet versions live in `Directory.Packages.props`. Never add `Version=` to a `<PackageReference>` in a `.csproj`.
|
|
|
|
---
|
|
|
|
## 13. Code organisation
|
|
|
|
- One type per file. File name matches the type name exactly.
|
|
- Handlers and validators go in the same feature folder — not in separate `Handlers/` or `Validators/` root folders.
|
|
- If a file exceeds ~150 lines, consider splitting it. Long files usually mean mixed concerns.
|
|
- Partial classes are only for generated code (source generators, EF scaffolding).
|
|
- Keep `Program.cs` as an orchestrator — extension method calls only, no logic.
|