diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 9a4225f..678aa72 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 08fb74d..981d60b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e46e622..f1ca5bd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -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. diff --git a/README.md b/README.md index 7a62d69..99079ea 100644 --- a/README.md +++ b/README.md @@ -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) @@ -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. diff --git a/adr/0001-adopt-traditional-layered-architecture.md b/adr/0001-adopt-traditional-layered-architecture.md new file mode 100644 index 0000000..c764e2f --- /dev/null +++ b/adr/0001-adopt-traditional-layered-architecture.md @@ -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. diff --git a/adr/0002-use-mvc-controllers-over-minimal-api.md b/adr/0002-use-mvc-controllers-over-minimal-api.md new file mode 100644 index 0000000..38eb86a --- /dev/null +++ b/adr/0002-use-mvc-controllers-over-minimal-api.md @@ -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. diff --git a/adr/0003-use-sqlite-for-data-storage.md b/adr/0003-use-sqlite-for-data-storage.md new file mode 100644 index 0000000..efcede7 --- /dev/null +++ b/adr/0003-use-sqlite-for-data-storage.md @@ -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. diff --git a/adr/0004-use-uuid-as-database-primary-key.md b/adr/0004-use-uuid-as-database-primary-key.md new file mode 100644 index 0000000..ec603e4 --- /dev/null +++ b/adr/0004-use-uuid-as-database-primary-key.md @@ -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. diff --git a/adr/0005-use-squad-number-as-api-mutation-key.md b/adr/0005-use-squad-number-as-api-mutation-key.md new file mode 100644 index 0000000..d6d1d87 --- /dev/null +++ b/adr/0005-use-squad-number-as-api-mutation-key.md @@ -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. diff --git a/adr/0006-use-rfc-7807-problem-details-for-errors.md b/adr/0006-use-rfc-7807-problem-details-for-errors.md new file mode 100644 index 0000000..d435544 --- /dev/null +++ b/adr/0006-use-rfc-7807-problem-details-for-errors.md @@ -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. diff --git a/adr/0007-use-fluentvalidation-over-data-annotations.md b/adr/0007-use-fluentvalidation-over-data-annotations.md new file mode 100644 index 0000000..826510d --- /dev/null +++ b/adr/0007-use-fluentvalidation-over-data-annotations.md @@ -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()`. 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. diff --git a/adr/0008-use-automapper-for-dto-mapping.md b/adr/0008-use-automapper-for-dto-mapping.md new file mode 100644 index 0000000..20339be --- /dev/null +++ b/adr/0008-use-automapper-for-dto-mapping.md @@ -0,0 +1,32 @@ +# 0008. Use AutoMapper for DTO Mapping + +Date: 2026-04-02 + +## Status + +Accepted + +## Context + +The API separates its public contract (request/response DTOs) from its internal persistence model (`Player` entity). Every controller action that reads or writes data must map between these representations. Without a mapping library, this means writing explicit property-assignment code (`responseModel.Name = entity.Name`, etc.) in every service method — boilerplate that grows linearly with the number of fields and operations. + +AutoMapper provides convention-based mapping: if property names match between source and destination, the mapping is configured once in a profile and applied everywhere. Custom mappings (renaming, transforming, ignoring fields) are expressed declaratively in the profile class. + +## Decision + +We will use AutoMapper for all DTO-to-entity and entity-to-DTO mappings. Mappings are defined in `PlayerMappingProfile` under `Mappings/` and registered via `AddAutoMapper(typeof(PlayerMappingProfile))`. `PlayerRequestModel` does not expose an `Id` property, so callers structurally cannot set or override the internal primary key — no explicit `.ForMember(...Ignore())` is required on the `PlayerRequestModel → Player` mapping. The `Player → PlayerResponseModel` mapping suppresses the AutoMapper validation warning for the unmapped `Id` source member via `.ForSourceMember(source => source.Id, options => options.DoNotValidate())`. + +## Consequences + +### Positive +- Eliminates repetitive property-assignment boilerplate in service methods. +- Mapping configuration is centralised in one profile class, making it easy to audit what is and is not mapped. +- Protection of the internal `Id` is structural — the absence of `Id` on `PlayerRequestModel` is the guarantee, which is simpler and more robust than relying on an explicit ignore rule. + +### Negative +- AutoMapper uses reflection at startup to validate mapping configurations, adding a small startup overhead. +- "Magic" convention-based mapping can be opaque: when a property is silently unmapped due to a name mismatch, the bug only surfaces at runtime (though `AssertConfigurationIsValid()` in tests can catch this). +- Adding AutoMapper as a dependency means contributors must understand profile configuration to modify or extend mappings. + +### Neutral +- AutoMapper's `ProjectTo()` LINQ extension for EF Core queryable projections is available but not currently used. Direct entity retrieval followed by in-memory mapping is sufficient at this project's scale. diff --git a/adr/0009-implement-in-memory-caching.md b/adr/0009-implement-in-memory-caching.md new file mode 100644 index 0000000..4b9424c --- /dev/null +++ b/adr/0009-implement-in-memory-caching.md @@ -0,0 +1,34 @@ +# 0009. Implement In-Memory Caching + +Date: 2026-04-02 + +## Status + +Accepted + +## Context + +The player list endpoint (`GET /players`) queries the database on every request. For a read-heavy API where the data changes infrequently, this is unnecessary load. Caching the result reduces database round-trips and demonstrates a common performance pattern for learners. + +The two main caching approaches in .NET are `IMemoryCache` (process-local, in-memory) and distributed caching (Redis, SQL Server — shared across multiple instances via `IDistributedCache`). Distributed caching is appropriate for multi-instance deployments where cache consistency across nodes matters. + +This project runs as a single instance (local development or a single Docker container) with no horizontal scaling requirement. + +## Decision + +We will use `IMemoryCache` managed in the service layer (`PlayerService`). Cache entries use a 10-minute sliding expiration combined with a one-hour absolute expiration: each access within the sliding window resets the TTL, but the entry is always evicted after one hour regardless of access frequency. On a cache miss, the service queries the repository, stores the result under a well-known key with these options, and returns it. Write operations (create, update, delete) invalidate the cache entry so the next read reflects the current state. + +## Consequences + +### Positive +- Zero additional infrastructure: `IMemoryCache` is built into `Microsoft.Extensions.Caching.Memory` with no external service dependency. +- The service layer caching pattern is easy to follow and demonstrates the cache-aside pattern clearly for learners. +- Reduces database queries for the most frequent read operation without any changes to the repository or controller layers. + +### Negative +- Cache state is lost on every application restart, so the first request after a restart always hits the database. +- `IMemoryCache` is not shared across multiple instances. If this application were scaled horizontally, each instance would maintain its own independent cache, potentially serving stale or inconsistent data across nodes. +- Memory usage grows with the size of the cached data; unbounded caching of large datasets in a long-running process could contribute to memory pressure. + +### Neutral +- If the project ever moves to a multi-instance deployment (e.g., Kubernetes), replacing `IMemoryCache` with a Redis-backed `IDistributedCache` would require changes only in the service layer and `Program.cs` registration — the repository and controller layers are unaffected. diff --git a/adr/0010-use-serilog-for-structured-logging.md b/adr/0010-use-serilog-for-structured-logging.md new file mode 100644 index 0000000..13af9f6 --- /dev/null +++ b/adr/0010-use-serilog-for-structured-logging.md @@ -0,0 +1,34 @@ +# 0010. Use Serilog for Structured Logging + +Date: 2026-04-02 + +## Status + +Accepted + +## Context + +.NET provides a built-in logging abstraction (`Microsoft.Extensions.Logging`) with pluggable providers. Out of the box it supports console and debug output, but structured log output — where log events are emitted as queryable key-value pairs rather than plain strings — requires a third-party sink. + +Structured logging is important even for a learning project because it demonstrates a production practice: log consumers (Elasticsearch, Seq, Datadog) ingest structured events and can filter, aggregate, and alert on specific fields. Emitting plain strings that embed values in a message template forfeits that capability. + +Serilog is the most widely used structured logging library in the .NET ecosystem. It integrates with `ILogger` via a host-level configuration, meaning application code never imports Serilog directly — it depends only on the framework abstraction. + +## Decision + +We will use Serilog, configured at host level in `Program.cs` via `UseSerilog()`. Two sinks are configured: console (for local development visibility) and file (for persistent log output under `logs/`). Message template parameters are passed as structured properties (`{@Player}`, `{SquadNumber}`), not interpolated strings, so that sinks that support structured output can index them as discrete fields. + +## Consequences + +### Positive +- Application code depends on `ILogger` (the framework abstraction), not on Serilog directly. Swapping the logging backend requires only a `Program.cs` change. +- Structured log events support downstream querying and alerting in log management systems, demonstrating a production-grade logging practice. +- Serilog's enrichers (machine name, thread ID, environment) can be added declaratively in configuration without touching application code. +- The file sink provides a persistent log archive useful for debugging Docker container runs after the container exits. + +### Negative +- Serilog is an additional dependency. Its host-level integration (`UseSerilog`) replaces the default ASP.NET Core logging pipeline, which can surprise contributors who expect the default provider behaviour. +- Misconfigured message templates (e.g., using string interpolation instead of structured parameters) silently degrade structured output without compile-time warnings. + +### Neutral +- The project previously used `Microsoft.Extensions.Logging` directly (removed in PR #192). The migration to Serilog was driven by the need for file sink support and consistent structured output across the cross-language comparison set. diff --git a/adr/0011-use-docker-for-containerization.md b/adr/0011-use-docker-for-containerization.md new file mode 100644 index 0000000..772e5bd --- /dev/null +++ b/adr/0011-use-docker-for-containerization.md @@ -0,0 +1,33 @@ +# 0011. Use Docker for Containerization + +Date: 2026-04-02 + +## Status + +Accepted + +## Context + +The project needs to be runnable without requiring contributors to install the .NET SDK, configure environment variables, or manage database files manually. A containerised delivery format solves the "works on my machine" problem and demonstrates a standard deployment practice. + +Docker is the de facto containerisation standard in the .NET ecosystem and across the cross-language comparison set that this project belongs to. Docker Compose provides a single-command local orchestration layer that handles image building, port mapping, and volume mounting without requiring knowledge of raw `docker run` flags. + +## Decision + +We will provide a multi-stage `Dockerfile` (build stage + runtime stage) and a `compose.yaml` for local orchestration. The application runs on port 9000 in the container, matching the local development port. A named Docker volume persists the SQLite database file across container restarts. The first run copies a pre-seeded database into the volume; subsequent runs reuse the existing volume. + +## Consequences + +### Positive +- `docker compose up` is the only command needed to run the full application from a fresh clone, with no SDK or database setup required. +- Multi-stage builds produce a minimal runtime image: only the published application artifacts are included, not the SDK or intermediate build outputs. +- The containerised environment closely mirrors what the CD pipeline builds and publishes to GitHub Container Registry, reducing environment-specific surprises. +- Port 9000 is consistent across all environments (local, Docker, CI), eliminating port-related configuration drift. + +### Negative +- Docker Desktop is required on developer machines. On macOS and Windows, Docker Desktop is a large install with licensing implications for commercial use. +- The Docker abstraction layer adds a level of indirection that can make debugging harder for contributors unfamiliar with container networking or volume mounts. +- SQLite inside a Docker volume is not suitable for production use — it is an appropriate trade-off only because this project is explicitly a development and learning reference. + +### Neutral +- Images are published to GitHub Container Registry (`ghcr.io`) as part of the CD pipeline, tagged by semantic version, stadium name, and `latest`. See the Releases section of `README.md` for the full tagging convention. diff --git a/adr/0012-use-stadium-themed-semantic-versioning.md b/adr/0012-use-stadium-themed-semantic-versioning.md new file mode 100644 index 0000000..9f09141 --- /dev/null +++ b/adr/0012-use-stadium-themed-semantic-versioning.md @@ -0,0 +1,35 @@ +# 0012. Use Stadium-Themed Semantic Versioning + +Date: 2026-04-02 + +## Status + +Accepted + +## Context + +The project follows Semantic Versioning (`MAJOR.MINOR.PATCH`) for release numbers. Purely numeric version strings are accurate but forgettable — contributors and users rarely remember whether "2.1.0" came before or after "2.0.3" without consulting the changelog. + +The project is football-themed (it manages football player data), and is part of a cross-language comparison set. A naming convention that reflects the project's domain makes releases memorable and reinforces the project's identity. + +Several well-known projects use codename conventions: Ubuntu (alphabetical adjective-animal pairs), Android (alphabetical desserts until Android 10), macOS (California landmarks). The pattern is established and understood. + +## Decision + +We will append a football stadium codename to every release tag, following the format `vMAJOR.MINOR.PATCH-stadium` (e.g., `v2.1.0-dusseldorf`). Stadium names are drawn from a fixed, alphabetically ordered list of venues that hosted FIFA World Cup matches, documented in `CHANGELOG.md`. Names are assigned sequentially A→Z; the next release always uses the next unused letter. The tag format is enforced by the CD pipeline, which validates the stadium name before publishing. + +## Consequences + +### Positive +- Release names are memorable: "the dusseldorf release" is easier to recall and discuss than "2.1.0". +- Alphabetical ordering provides an implicit sequence — contributors can determine release order from the name alone without consulting version numbers. +- The convention is consistent and deterministic: there is no ambiguity about which name comes next. +- Reinforces the football domain theme throughout the project's lifecycle. + +### Negative +- The convention is non-standard and may confuse first-time contributors who expect plain semantic version tags. +- The stadium list is finite (26 letters). If the project ever reaches 26 major releases, the list must be extended or the convention revisited. +- The CD pipeline validation adds a small amount of CI complexity to enforce the format. + +### Neutral +- The stadium list is published in `CHANGELOG.md` under "Stadium Release Names" for reference. The current position in the sequence is always the last released tag — the next unused letter determines the next stadium name. diff --git a/adr/README.md b/adr/README.md new file mode 100644 index 0000000..e17f134 --- /dev/null +++ b/adr/README.md @@ -0,0 +1,62 @@ +# Architecture Decision Records + +This directory contains Architecture Decision Records (ADRs) for this project. ADRs document significant architectural decisions — what was decided, why, and what trade-offs were accepted. + +## Index + +| ADR | Title | Status | +|-----|-------|--------| +| [0001](0001-adopt-traditional-layered-architecture.md) | Adopt Traditional Layered Architecture | Accepted (Under Reconsideration) | +| [0002](0002-use-mvc-controllers-over-minimal-api.md) | Use MVC Controllers over Minimal API | Accepted | +| [0003](0003-use-sqlite-for-data-storage.md) | Use SQLite for Data Storage | Accepted | +| [0004](0004-use-uuid-as-database-primary-key.md) | Use UUID as Database Primary Key | Accepted | +| [0005](0005-use-squad-number-as-api-mutation-key.md) | Use Squad Number as API Mutation Key | Accepted | +| [0006](0006-use-rfc-7807-problem-details-for-errors.md) | Use RFC 7807 Problem Details for Errors | Accepted | +| [0007](0007-use-fluentvalidation-over-data-annotations.md) | Use FluentValidation over Data Annotations | Accepted | +| [0008](0008-use-automapper-for-dto-mapping.md) | Use AutoMapper for DTO Mapping | Accepted | +| [0009](0009-implement-in-memory-caching.md) | Implement In-Memory Caching | Accepted | +| [0010](0010-use-serilog-for-structured-logging.md) | Use Serilog for Structured Logging | Accepted | +| [0011](0011-use-docker-for-containerization.md) | Use Docker for Containerization | Accepted | +| [0012](0012-use-stadium-themed-semantic-versioning.md) | Use Stadium-Themed Semantic Versioning | Accepted | + +## When to Create an ADR + +Create an ADR when a decision affects: + +- 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 + +## Template + +```markdown +# [NUMBER]. [TITLE] + +Date: YYYY-MM-DD + +## Status + +[Proposed | Accepted | Deprecated | Superseded by ADR-XXXX] + +## Context + +What is the issue we are facing? What forces are at play (technical, political, +social, project constraints)? Present facts neutrally without bias. + +## Decision + +We will [DECISION IN ACTIVE VOICE WITH FULL SENTENCES]. + +## Consequences + +### Positive +- Consequence 1 + +### Negative +- Trade-off 1 + +### Neutral +- Other effect 1 +```