Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,15 @@ This project uses Spec-Driven Development (SDD): discuss in Plan mode first, cre

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

## Architecture Decision Records (ADRs)

Load `#file:adr/README.md` when:
- The user asks about architectural choices or "why we use X"
- Proposing changes to core architecture or dependencies
- Historical context for past decisions is needed

ADRs are in `adr/` (0001–0012). Each file is self-contained.

**After completing work**: Suggest a branch name (e.g. `feat/add-player-search`) and a commit message following Conventional Commits including co-author line:

```text
Expand Down
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ This project uses famous football stadiums (A-Z) that hosted FIFA World Cup matc

### Added

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

### Changed

### Fixed
Expand Down
14 changes: 13 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,19 @@ We enforce quality via CI on every push and PR:

Failures must be fixed before review.

## 6. Code of Conduct & Support
## 6. Architecture Decision Records

When proposing changes that affect:

- Project structure or overall architecture
- Technology choices or dependencies
- API contracts or data model design
- Non-functional requirements (performance, security, scalability)
- Development workflows or processes

Please create an ADR in the `adr/` directory following the existing template. See [`adr/README.md`](adr/README.md) for the template and index.

## 7. Code of Conduct & Support

- Please see `CODE_OF_CONDUCT.md` for behavioral expectations and reporting.
- For quick questions or discussions, open an issue with the `discussion` label or mention a maintainer.
Expand Down
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Proof of Concept for a RESTful API built with .NET 10 (LTS) and ASP.NET Core. Ma
- [Tech Stack](#tech-stack)
- [Project Structure](#project-structure)
- [Architecture](#architecture)
- [Architecture Decisions](#architecture-decisions)
- [API Reference](#api-reference)
- [Prerequisites](#prerequisites)
- [Quick Start](#quick-start)
Expand Down Expand Up @@ -217,6 +218,10 @@ Blue = core application packages, yellow = Microsoft platform packages, red = th

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

## Architecture Decisions

See [Architecture Decision Records (ADRs)](adr/README.md) for documented decisions about this project's architecture, technology choices, and development practices.

## API Reference

Interactive API documentation is available via Swagger UI at `https://localhost:9000/swagger/index.html` when the server is running.
Expand Down
32 changes: 32 additions & 0 deletions adr/0001-adopt-traditional-layered-architecture.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# 0001. Adopt Traditional Layered Architecture

Date: 2026-04-02

## Status

Accepted (Under Reconsideration — see [Issue #266](https://github.com/nanotaboada/Dotnet.Samples.AspNetCore.WebApi/issues/266))

## Context

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.

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.

## Decision

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.

## Consequences

### Positive
- The pattern is widely understood and requires no prior knowledge of advanced architectural styles.
- The folder structure maps directly to responsibilities, making the codebase easy to navigate.
- Onboarding friction is low — contributors can locate any piece of logic by layer name alone.

### Negative
- Domain entities (`Player`) are shared across all layers, mixing persistence concerns with domain logic.
- Dependencies flow in multiple directions at the model level, limiting flexibility.
- Tight coupling between layers makes large-scale refactoring harder as the project grows.

### Neutral
- 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.
33 changes: 33 additions & 0 deletions adr/0002-use-mvc-controllers-over-minimal-api.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# 0002. Use MVC Controllers over Minimal API

Date: 2026-04-02

## Status

Accepted

## Context

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.

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.

## Decision

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.

## Consequences

### Positive
- Controllers provide explicit, visible grouping of related endpoints in a single class.
- Constructor injection and interface-based dependencies make controllers straightforward to unit test with mocks.
- The pattern is familiar to developers coming from any MVC background (Spring MVC, Django CBVs, Rails controllers), lowering the barrier for the target audience.
- Attribute routing (`[HttpGet]`, `[HttpPost]`, etc.) keeps route definitions co-located with their handlers.

### Negative
- Controllers introduce more boilerplate than Minimal API: class declarations, action method signatures, and `[FromRoute]`/`[FromBody]` attributes.
- Minimal API is the direction Microsoft is actively investing in for new projects; staying on MVC controllers means diverging from that trajectory over time.
- The framework overhead of MVC (model binding pipeline, action filters, etc.) is larger than Minimal API, though negligible at this project's scale.

### Neutral
- 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.
32 changes: 32 additions & 0 deletions adr/0003-use-sqlite-for-data-storage.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# 0003. Use SQLite for Data Storage

Date: 2026-04-02

## Status

Accepted

## Context

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.

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.

## Decision

We will use SQLite as the database engine, accessed through Entity Framework Core. The database file is stored at `storage/players-sqlite3.db` and is pre-seeded with sample data. Docker deployments mount the file into a named volume so data survives container restarts.

## Consequences

### Positive
- Zero-config: no server process, no connection string credentials, no Docker service dependency for local development.
- The database file can be committed to the repository as seed data, making onboarding instant.
- EF Core abstracts the SQL dialect, so migrating to another database requires changing only the provider registration.

### Negative
- SQLite does not support concurrent writes, making it unsuitable for multi-instance deployments or high-throughput scenarios.
- Some EF Core features (e.g., certain migration operations) behave differently with the SQLite provider.
- Not representative of a production database choice, which may mislead learners about real-world persistence decisions.

### Neutral
- 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.
32 changes: 32 additions & 0 deletions adr/0004-use-uuid-as-database-primary-key.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# 0004. Use UUID as Database Primary Key

Date: 2026-04-02

## Status

Accepted

## Context

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.

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.

## Decision

We will use `Guid` (`UUID v4`) as the primary key for the `Player` entity. The `Player` model declares `public Guid Id { get; set; } = Guid.NewGuid()`, which generates the value at the application layer before EF Core calls `SaveChanges`. `PlayerDbContext` additionally configures `entity.Property(player => player.Id).ValueGeneratedOnAdd()`, which signals to EF Core that the column is auto-generated — this is consistent with the field initializer approach and ensures migrations reflect the intended behaviour. The `Id` property is never included in API request or response models.

## Consequences

### Positive
- Opaque IDs prevent callers from guessing or enumerating records by probing sequential values.
- The field initializer (`Guid.NewGuid()`) generates the UUID at the application layer before the record reaches the database, decoupling identity generation from the persistence engine.
- Aligns with .NET defaults: `Guid.NewGuid()` requires no additional libraries or configuration.

### Negative
- `Guid` occupies 16 bytes vs 4 bytes for `int`, increasing index and storage size.
- 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.
- UUIDs are not human-readable, making raw database inspection or log correlation harder.

### Neutral
- 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.
34 changes: 34 additions & 0 deletions adr/0005-use-squad-number-as-api-mutation-key.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# 0005. Use Squad Number as API Mutation Key

Date: 2026-04-02

## Status

Accepted

## Context

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`).

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.

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

## Decision

We will use `squadNumber` as the route parameter for mutation endpoints and primary lookup routes: `GET /players/squadNumber/{n}`, `PUT /players/squadNumber/{n}`, and `DELETE /players/squadNumber/{n}`. The internal `Id` (UUID) is never included in any request model or response model. A secondary `GET /players/{id:Guid}` route exists for internal or administrative lookups by UUID but is not part of the primary API contract.

## Consequences

### Positive
- URLs are human-readable and predictable from domain knowledge alone (`/players/squadNumber/10` unambiguously refers to the number-10 player).
- API consumers are fully decoupled from the internal persistence model.
- 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.

### Negative
- 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.
- 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.
- `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.

### Neutral
- The `GET /players/{id:Guid}` endpoint exists as a secondary route for internal or administrative lookups by UUID. It is intentionally excluded from the primary API contract and is not the recommended path for general consumers.
32 changes: 32 additions & 0 deletions adr/0006-use-rfc-7807-problem-details-for-errors.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# 0006. Use RFC 7807 Problem Details for Errors

Date: 2026-04-02

## Status

Accepted

## Context

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.

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

## Decision

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.

## Consequences

### Positive
- A single, predictable error shape across all endpoints simplifies client-side error handling — consumers check `status` and `detail` without branching on response format.
- 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.
- Built-in ASP.NET Core support means no custom serialisation code is needed.
- Validation errors include field-level detail via the `errors` dictionary, giving consumers enough context to display meaningful feedback.

### Negative
- The response payload is more verbose than a plain string message, which adds minor overhead for simple error cases.
- Consumers must understand the Problem Details schema to interpret `type` URIs and distinguish error categories beyond HTTP status codes.

### Neutral
- The `type` field conventionally holds a URI that identifies the problem type. Most responses rely on the ASP.NET Core default, but individual controllers may override it with problem-specific URIs where appropriate — for example, `PlayerController` sets `Type = "https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/409"` for conflict responses. In a production API, all `type` URIs would point to project-owned documentation pages.
35 changes: 35 additions & 0 deletions adr/0007-use-fluentvalidation-over-data-annotations.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# 0007. Use FluentValidation over Data Annotations

Date: 2026-04-02

## Status

Accepted

## Context

ASP.NET Core provides two primary mechanisms for validating request models: Data Annotations (attributes applied directly to model properties, e.g. `[Required]`, `[Range]`) and FluentValidation (a separate library that defines validation rules in dedicated validator classes).

Data Annotations are built into the framework and require no additional dependencies, but they are limited to declarative, property-level rules. Complex cross-property validation, async database checks (e.g., verifying that a squad number is unique), and operation-specific rules (different rules for create vs. update) are difficult or impossible to express with annotations alone.

The player creation flow requires an async uniqueness check against the database (`BeUniqueSquadNumber`). The update flow must skip that same check because the player already exists. This kind of operation-contextual validation is a first-class concern in FluentValidation via named rule sets.

## Decision

We will use FluentValidation for all request model validation. Validators are defined in dedicated classes under `Validators/` and registered with the DI container via `AddValidatorsFromAssemblyContaining<PlayerRequestModelValidator>()`. Validation rules are grouped into named rule sets (`Create` and `Update`) to make operation-specific behaviour explicit. Controllers invoke the appropriate rule set by name.

## Consequences

### Positive
- Validation logic lives in its own class, separate from the model and the controller, respecting the Single Responsibility Principle.
- Named rule sets (`Create`, `Update`) make it explicit which rules apply to which HTTP operations, eliminating hidden branching inside the validator.
- Async rules (e.g., `MustAsync(BeUniqueSquadNumber)`) integrate naturally, enabling database-backed validation without awkward workarounds.
- Validators are plain classes and trivial to unit test in isolation.

### Negative
- FluentValidation is an additional NuGet dependency not bundled with the framework.
- The `ValidateAsync` overload called internally is the `IValidationContext` overload, not the generic one — an implementation detail that must be respected when mocking validators in controller tests (see Copilot instructions for the correct Moq setup).
- Teams unfamiliar with FluentValidation face a learning curve around rule sets and the async API.

### Neutral
- FluentValidation does not replace model binding. Invalid JSON structure or type mismatches are still caught by the ASP.NET Core model binder before FluentValidation runs.
Loading
Loading