Skip to content

Add claude.md with project conventions and build instructions #93

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Giorgi wants to merge 3 commits into from
Closed

Add claude.md with project conventions and build instructions #93

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d972059
Add claude.md with project conventions and build instructions
claude Feb 12, 2026
8a0db27
Address PR feedback: fix test command and SQLite Docker note
claude Feb 12, 2026
7f8d97d
Add test for PostgreSQL SqlState 22001 max length violation (fixes #92)
claude Feb 12, 2026
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
51 changes: 51 additions & 0 deletions claude.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# EntityFramework.Exceptions

Typed exception handling for Entity Framework Core. Converts database-specific errors into strongly-typed .NET exceptions instead of generic `DbUpdateException`.

## Build & Test

```bash
dotnet restore # Restore NuGet dependencies
dotnet build --no-restore # Build all projects
dotnet test --no-build # Run tests (requires Docker for Testcontainers)
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dotnet test --no-build assumes the test projects have already been built. Since the preceding command uses dotnet build (not dotnet build -t:Build on the solution including tests), this can lead to test failures on a clean checkout. Consider changing the test command to dotnet test --no-restore (or drop --no-build) to ensure tests build as needed.

Suggested change
dotnet test --no-build # Run tests (requires Docker for Testcontainers)
dotnet test --no-restore # Run tests (requires Docker for Testcontainers)

Copilot uses AI. Check for mistakes.
```

Tests use **Testcontainers** and require a running Docker daemon. Each database provider (SQL Server, PostgreSQL, MySQL, Oracle, SQLite) spins up its own container.
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence states SQLite spins up its own container, but SQLite is typically in-process and does not require Docker. Please correct the wording to avoid misleading setup requirements (e.g., list only providers that use containers and call out SQLite as non-containerized).

Suggested change
Tests use **Testcontainers** and require a running Docker daemon. Each database provider (SQL Server, PostgreSQL, MySQL, Oracle, SQLite) spins up its own container.
Tests use **Testcontainers** and require a running Docker daemon for containerized providers (SQL Server, PostgreSQL, MySQL, Oracle), each of which spins up its own container. **SQLite** tests run in-process and do not require Docker.

Copilot uses AI. Check for mistakes.

## Project Structure

The solution (`EntityFramework.Exceptions.slnx`) has two main layers:
Comment thread
Giorgi marked this conversation as resolved.

- **DbExceptionClassifier/** — Database-specific error code classification. Each provider implements `IDbExceptionClassifier` to map native error codes to a `DatabaseError` enum.
- `Common/` — `IDbExceptionClassifier` interface
- `SqlServer/`, `PostgreSQL/`, `MySQL/`, `MySQL.Pomelo/`, `Oracle/`, `Sqlite/` — Provider implementations

- **EntityFramework.Exceptions/** — EF Core integration via interceptors. Catches `DbException`, classifies it, and throws a typed exception.
- `Common/` — Base `ExceptionProcessorInterceptor<T>`, exception classes (`UniqueConstraintException`, `CannotInsertNullException`, `MaxLengthExceededException`, `NumericOverflowException`, `ReferenceConstraintException`), `ExceptionFactory`
- `SqlServer/`, `PostgreSQL/`, `MySQL/`, `MySQL.Pomelo/`, `Oracle/`, `Sqlite/` — Provider-specific interceptors and `UseExceptionProcessor()` extension methods
- `Tests/` — xUnit test suite using Testcontainers

- **Directory.Build.props** — Shared build properties (target framework, version, NuGet metadata). All non-Common projects automatically reference their corresponding Common project.

## Architecture

1. **Interceptor pattern**: `ExceptionProcessorInterceptor<TProviderException>` implements `IDbCommandInterceptor` and `ISaveChangesInterceptor`
2. **Classification**: Each database provider has an `IDbExceptionClassifier` that maps native error codes to `DatabaseError` enum values
3. **Factory**: `ExceptionFactory` creates the appropriate typed exception
4. **Extension methods**: Each provider exposes `UseExceptionProcessor()` on `DbContextOptionsBuilder`

## Code Conventions

- **C# / .NET 8.0** with file-scoped namespaces, primary constructors, nullable reference types, and implicit usings
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The statement mentions "implicit usings" as a code convention, but the test project (Tests.csproj) does not have ImplicitUsings enabled. Evidence: explicit using statements are still required in test files (e.g., System.Threading.Tasks was added in PostgreSQLTests.cs), and the Tests.csproj file doesn't contain an ImplicitUsings property. Consider removing "implicit usings" from this list or clarifying that it applies only to certain projects.

Suggested change
- **C# / .NET 8.0** with file-scoped namespaces, primary constructors, nullable reference types, and implicit usings
- **C# / .NET 8.0** with file-scoped namespaces, primary constructors, nullable reference types; implicit usings are enabled in library projects, while test projects use explicit `using` directives

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The statement mentions "nullable reference types" as a code convention, but none of the csproj files examined (Directory.Build.props, Tests.csproj, PostgreSQL.csproj) contain a Nullable property set to "enable". In .NET 8.0, nullable reference types must be explicitly enabled via the Nullable property in the project file. Consider removing "nullable reference types" from this list or clarifying which projects have this enabled, if any.

Suggested change
- **C# / .NET 8.0** with file-scoped namespaces, primary constructors, nullable reference types, and implicit usings
- **C# / .NET 8.0** with file-scoped namespaces, primary constructors, and implicit usings

Copilot uses AI. Check for mistakes.
- **Naming**: `[Database]ExceptionClassifier`, `[Database]ExceptionProcessorInterceptor`, `ExceptionProcessorExtensions.UseExceptionProcessor()`
- **Test naming**: `[Scenario]Throws[ExceptionType]` (e.g., `UniqueColumnViolationThrowsUniqueConstraintException`)
- **MySQL.Pomelo** shares source files with **MySQL** via `<Link>` in .csproj and uses `#if POMELO` preprocessor directive
- Exception classes follow a standard pattern: inherit `DbUpdateException`, provide all standard constructor overloads, and optionally expose `ConstraintName`, `ConstraintProperties`, and `SchemaQualifiedTableName` properties

## Testing Notes

- Base test class `DatabaseTests` defines ~12 virtual test methods; provider-specific test classes inherit and override as needed
- **SQLite** does not populate `ConstraintName`/`ConstraintProperties` and does not enforce numeric overflow
- **SQL Server** skips numeric overflow tests (`ArgumentException` from SqlClient)
- **MySQL** primary key violations do not populate constraint properties
- Test fixtures use `IAsyncLifetime` for container lifecycle management
Loading