Skip to content

Commit b6d6fd6

Browse files
nanotaboadaclaude
andcommitted
docs(adr): add Architecture Decision Records (#372)
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent a6e8f12 commit b6d6fd6

17 files changed

Lines changed: 493 additions & 1 deletion

.github/copilot-instructions.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,15 @@ This project uses Spec-Driven Development (SDD): discuss in Plan mode first, cre
208208

209209
**Modify schema**: Update `Player` entity → update DTOs → update AutoMapper profile → reset `Storage/players.db` → update tests → run `dotnet test`.
210210

211+
## Architecture Decision Records (ADRs)
212+
213+
Load `#file:adr/README.md` when:
214+
- The user asks about architectural choices or "why we use X"
215+
- Proposing changes to core architecture or dependencies
216+
- Historical context for past decisions is needed
217+
218+
ADRs are in `adr/` (0001–0012). Each file is self-contained.
219+
211220
**After completing work**: Suggest a branch name (e.g. `feat/add-player-search`) and a commit message following Conventional Commits including co-author line:
212221

213222
```text

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ This project uses famous football stadiums (A-Z) that hosted FIFA World Cup matc
4444

4545
### Added
4646

47+
- Add `adr/` directory with 12 Architecture Decision Records documenting architectural choices, technology decisions, and design trade-offs (#372)
48+
- Add ADR index and template at `adr/README.md` (#372)
49+
- Add Architecture Decisions section to `README.md` referencing the ADR index (#372)
50+
- Add ADR guidance section to `CONTRIBUTING.md` (#372)
51+
- Add ADR context loading instructions to `.github/copilot-instructions.md` (#372)
52+
4753
### Changed
4854

4955
### Fixed

CONTRIBUTING.md

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,19 @@ We enforce quality via CI on every push and PR:
4444

4545
Failures must be fixed before review.
4646

47-
## 6. Code of Conduct & Support
47+
## 6. Architecture Decision Records
48+
49+
When proposing changes that affect:
50+
51+
- Project structure or overall architecture
52+
- Technology choices or dependencies
53+
- API contracts or data model design
54+
- Non-functional requirements (performance, security, scalability)
55+
- Development workflows or processes
56+
57+
Please create an ADR in the `adr/` directory following the existing template. See [`adr/README.md`](adr/README.md) for the template and index.
58+
59+
## 7. Code of Conduct & Support
4860

4961
- Please see `CODE_OF_CONDUCT.md` for behavioral expectations and reporting.
5062
- For quick questions or discussions, open an issue with the `discussion` label or mention a maintainer.

README.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ Proof of Concept for a RESTful API built with .NET 10 (LTS) and ASP.NET Core. Ma
2020
- [Tech Stack](#tech-stack)
2121
- [Project Structure](#project-structure)
2222
- [Architecture](#architecture)
23+
- [Architecture Decisions](#architecture-decisions)
2324
- [API Reference](#api-reference)
2425
- [Prerequisites](#prerequisites)
2526
- [Quick Start](#quick-start)
@@ -217,6 +218,10 @@ Blue = core application packages, yellow = Microsoft platform packages, red = th
217218

218219
*Simplified, conceptual view — not all components or dependencies are shown.*
219220

221+
## Architecture Decisions
222+
223+
See [Architecture Decision Records (ADRs)](adr/README.md) for documented decisions about this project's architecture, technology choices, and development practices.
224+
220225
## API Reference
221226

222227
Interactive API documentation is available via Swagger UI at `https://localhost:9000/swagger/index.html` when the server is running.
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# 0001. Adopt Traditional Layered Architecture
2+
3+
Date: 2026-04-02
4+
5+
## Status
6+
7+
Accepted (Under Reconsideration — see [Issue #266](https://github.com/nanotaboada/Dotnet.Samples.AspNetCore.WebApi/issues/266))
8+
9+
## Context
10+
11+
The project needed a structural pattern to organise the codebase for a REST API managing football player data. The primary goal is educational clarity — the project is a learning reference, so the pattern must be immediately understandable to developers familiar with standard layered architecture.
12+
13+
The two main candidates considered were traditional layered architecture (Controllers → Services → Repositories → Data) and Clean Architecture (with explicit domain, application, and infrastructure rings). Clean Architecture offers better separation of concerns and testability at the cost of significantly more boilerplate and conceptual overhead for a small PoC.
14+
15+
## Decision
16+
17+
We will organise the codebase into four layers: HTTP (`Controllers`, `Validators`), Business (`Services`, `Mappings`), Data (`Repositories`, `Data`), and a cross-cutting `Models` layer. Each layer depends only on the layer below it; controllers must not access repositories directly, and business logic must not live in controllers.
18+
19+
## Consequences
20+
21+
### Positive
22+
- The pattern is widely understood and requires no prior knowledge of advanced architectural styles.
23+
- The folder structure maps directly to responsibilities, making the codebase easy to navigate.
24+
- Onboarding friction is low — contributors can locate any piece of logic by layer name alone.
25+
26+
### Negative
27+
- Domain entities (`Player`) are shared across all layers, mixing persistence concerns with domain logic.
28+
- Dependencies flow in multiple directions at the model level, limiting flexibility.
29+
- Tight coupling between layers makes large-scale refactoring harder as the project grows.
30+
31+
### Neutral
32+
- This decision is expected to be superseded when Issue #266 (Clean Architecture migration) is implemented. This ADR will remain in the repository as a record of the original decision and its context.
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# 0002. Use MVC Controllers over Minimal API
2+
3+
Date: 2026-04-02
4+
5+
## Status
6+
7+
Accepted
8+
9+
## Context
10+
11+
ASP.NET Core offers two approaches for building HTTP endpoints: traditional MVC controllers (attribute-routed classes inheriting from `ControllerBase`) and Minimal API (lambda-based route handlers registered directly in `Program.cs`). Microsoft has been investing in Minimal API as the preferred path for new greenfield services since .NET 6.
12+
13+
For this project, the primary concern is educational clarity. The codebase serves as a learning reference for developers exploring REST API patterns in .NET, and it is part of a cross-language comparison set where consistent structural conventions across stacks matter.
14+
15+
## Decision
16+
17+
We will use MVC controllers with attribute routing. Each controller is a class that encapsulates related HTTP handlers, receives its dependencies via constructor injection, and delegates all business logic to the service layer.
18+
19+
## Consequences
20+
21+
### Positive
22+
- Controllers provide explicit, visible grouping of related endpoints in a single class.
23+
- Constructor injection and interface-based dependencies make controllers straightforward to unit test with mocks.
24+
- The pattern is familiar to developers coming from any MVC background (Spring MVC, Django CBVs, Rails controllers), lowering the barrier for the target audience.
25+
- Attribute routing (`[HttpGet]`, `[HttpPost]`, etc.) keeps route definitions co-located with their handlers.
26+
27+
### Negative
28+
- Controllers introduce more boilerplate than Minimal API: class declarations, action method signatures, and `[FromRoute]`/`[FromBody]` attributes.
29+
- Minimal API is the direction Microsoft is actively investing in for new projects; staying on MVC controllers means diverging from that trajectory over time.
30+
- The framework overhead of MVC (model binding pipeline, action filters, etc.) is larger than Minimal API, though negligible at this project's scale.
31+
32+
### Neutral
33+
- This decision may be revisited if the project migrates to Clean Architecture (Issue #266), at which point Minimal API endpoints or vertical slice handlers could be a better fit for the new structure.
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# 0003. Use SQLite for Data Storage
2+
3+
Date: 2026-04-02
4+
5+
## Status
6+
7+
Accepted
8+
9+
## Context
10+
11+
The project requires a relational database to persist football player data. The main candidates were SQLite, PostgreSQL, and SQL Server. The project is a proof-of-concept and learning reference, not a production service. Deployment simplicity and zero-configuration setup are higher priorities than scalability or advanced database features.
12+
13+
The cross-language comparison set (Go/Gin, Java/Spring Boot, Python/FastAPI, Rust/Rocket, TypeScript/Node.js) all use SQLite for the same reasons, so consistency across the set also favours it.
14+
15+
## Decision
16+
17+
We will use SQLite as the database engine, accessed through Entity Framework Core. The database file is stored at `src/.../Storage/players.db` and is pre-seeded with sample data. Docker deployments mount the file into a named volume so data survives container restarts.
18+
19+
## Consequences
20+
21+
### Positive
22+
- Zero-config: no server process, no connection string credentials, no Docker service dependency for local development.
23+
- The database file can be committed to the repository as seed data, making onboarding instant.
24+
- EF Core abstracts the SQL dialect, so migrating to another database requires changing only the provider registration.
25+
26+
### Negative
27+
- SQLite does not support concurrent writes, making it unsuitable for multi-instance deployments or high-throughput scenarios.
28+
- Some EF Core features (e.g., certain migration operations) behave differently with the SQLite provider.
29+
- Not representative of a production database choice, which may mislead learners about real-world persistence decisions.
30+
31+
### Neutral
32+
- Issue #249 tracks adding PostgreSQL support for environments that require a production-grade database. When implemented, SQLite will remain the default for local development and this ADR will be supplemented by a new ADR documenting the PostgreSQL decision.
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# 0004. Use UUID as Database Primary Key
2+
3+
Date: 2026-04-02
4+
5+
## Status
6+
7+
Accepted
8+
9+
## Context
10+
11+
Every `Player` record requires a stable, unique identity for the database. The two common choices are a sequential integer (`int` / `IDENTITY`) and a UUID (`Guid`). Sequential integers are compact, human-readable, and index-friendly, but they leak information: an external caller who knows ID `42` can infer there are at least 42 records and probe adjacent IDs. UUIDs are opaque and decoupled from the database's row insertion order.
12+
13+
Crucially, the internal primary key is never surfaced in the API (see ADR 0005 for why `squadNumber` is the public-facing identifier). This means the choice of key type has no direct impact on API consumers.
14+
15+
## Decision
16+
17+
We will use `Guid` (`UUID v4`) as the primary key for the `Player` entity, generated at the application layer via `Guid.NewGuid()`. The `Id` property is never included in API request or response models and is intentionally excluded from the AutoMapper profile.
18+
19+
## Consequences
20+
21+
### Positive
22+
- Opaque IDs prevent callers from guessing or enumerating records by probing sequential values.
23+
- ID generation happens in application code, not the database, decoupling identity from the persistence engine and making it easier to change databases in the future.
24+
- Aligns with .NET defaults: `Guid.NewGuid()` requires no additional libraries or configuration.
25+
26+
### Negative
27+
- `Guid` occupies 16 bytes vs 4 bytes for `int`, increasing index and storage size.
28+
- Random UUID v4 values are not monotonically increasing, which can cause B-tree index fragmentation on write-heavy workloads. This is not a concern at this project's scale with SQLite, but would matter at production load on PostgreSQL or SQL Server.
29+
- UUIDs are not human-readable, making raw database inspection or log correlation harder.
30+
31+
### Neutral
32+
- Because `Id` is never exposed through the API, consumers are fully isolated from this decision. A future migration to sequential IDs or UUID v7 (time-ordered) would be a pure database/model change with no API contract impact.
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
# 0005. Use Squad Number as API Mutation Key
2+
3+
Date: 2026-04-02
4+
5+
## Status
6+
7+
Accepted
8+
9+
## Context
10+
11+
The API exposes endpoints for retrieving, updating, and deleting individual players. Each of these operations requires a route parameter that identifies the target player. Two candidates exist: the internal UUID primary key (`Id`) and the domain-meaningful squad number (`SquadNumber`).
12+
13+
Exposing the UUID would couple API consumers to an internal database concern. UUIDs carry no domain meaning — a caller who knows that player "Lionel Messi" wears number 10 cannot construct the URL from that knowledge alone; they would first need to discover the UUID from a prior `GET /players` call.
14+
15+
`SquadNumber` is the natural identifier for a football player within a team context. It is the value coaches, fans, and data consumers use to refer to players. It is also the field that other API operations (`POST` uniqueness check, `PUT` route matching) already reason about.
16+
17+
## Decision
18+
19+
We will use `squadNumber` as the route parameter for all single-resource endpoints: `GET /players/squadNumber/{n}`, `PUT /players/squadNumber/{n}`, and `DELETE /players/squadNumber/{n}`. The internal `Id` (UUID) is never included in any URL, request model, or response model.
20+
21+
## Consequences
22+
23+
### Positive
24+
- URLs are human-readable and predictable from domain knowledge alone (`/players/squadNumber/10` unambiguously refers to the number-10 player).
25+
- API consumers are fully decoupled from the internal persistence model.
26+
- Consistency: the same identifier used in the domain (`SquadNumber`) is used in the API surface, reducing the mental translation between domain concepts and HTTP calls.
27+
28+
### Negative
29+
- Squad numbers are unique only within a single team and only for the duration of a season. They can be reassigned when a player leaves or retires. This project models a single team's squad, so cross-team ambiguity is not a concern, but the limitation is real.
30+
- Uniqueness must be enforced at the application layer. The `BeUniqueSquadNumber` async validator rule in the `Create` rule set guards against duplicate squad numbers on insert; the `Update` rule set intentionally omits this check because the player already exists.
31+
- `squadNumber` is not a stable long-term identifier — if squad number reassignment were ever supported, consumers holding a bookmarked URL would silently reach a different player.
32+
33+
### Neutral
34+
- The `GET /players/{id:Guid}` endpoint (retrieve by internal UUID) exists as a separate route and is intended for internal or administrative use cases, not as the primary lookup mechanism.
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# 0006. Use RFC 7807 Problem Details for Errors
2+
3+
Date: 2026-04-02
4+
5+
## Status
6+
7+
Accepted
8+
9+
## Context
10+
11+
HTTP APIs need a consistent format for communicating errors to consumers. Without a standard, each endpoint might return a different error shape — plain strings, custom JSON objects, or raw status codes with empty bodies — making client-side error handling fragile and unpredictable.
12+
13+
RFC 7807 (Problem Details for HTTP APIs) defines a standard JSON structure for error responses with well-known fields (`type`, `title`, `status`, `detail`, `instance`). ASP.NET Core 7+ includes built-in support for this format via `TypedResults.Problem` and `TypedResults.ValidationProblem`.
14+
15+
## Decision
16+
17+
We will use RFC 7807 Problem Details for all error responses. Validation failures will use `TypedResults.ValidationProblem`, which extends the standard shape with a `errors` field containing field-level messages. All other errors (not found, conflict, internal server error) will use `TypedResults.Problem` with an appropriate HTTP status code and a human-readable `detail` field.
18+
19+
## Consequences
20+
21+
### Positive
22+
- A single, predictable error shape across all endpoints simplifies client-side error handling — consumers check `status` and `detail` without branching on response format.
23+
- The format is an IETF standard, meaning it is recognisable to developers and interoperable with API tooling, gateways, and monitoring systems that understand Problem Details.
24+
- Built-in ASP.NET Core support means no custom serialisation code is needed.
25+
- Validation errors include field-level detail via the `errors` dictionary, giving consumers enough context to display meaningful feedback.
26+
27+
### Negative
28+
- The response payload is more verbose than a plain string message, which adds minor overhead for simple error cases.
29+
- Consumers must understand the Problem Details schema to interpret `type` URIs and distinguish error categories beyond HTTP status codes.
30+
31+
### Neutral
32+
- The `type` field conventionally holds a URI that identifies the problem type. This project uses the ASP.NET Core default (`https://tools.ietf.org/html/rfc7807`), which is sufficient for a learning reference but would be replaced by project-specific URIs in a production API.

0 commit comments

Comments
 (0)