|
| 1 | +--- |
| 2 | +name: implementing-dapper-queries |
| 3 | +description: Implementing Dapper repository methods and stored procedures for MSSQL at Bitwarden. Use when creating or modifying Dapper repositories, writing stored procedures, or working with MSSQL-specific data access in the server repo. Also use when writing MSSQL migration scripts under `util/Migrator/DbScripts/` or touching SSDT schema under `src/Sql/dbo/`. |
| 4 | +--- |
| 5 | + |
| 6 | +## Repository Pattern |
| 7 | + |
| 8 | +All Dapper implementations live in `src/Infrastructure/Dapper/Repositories/`. Each repository class implements an interface from `src/Core/` and uses stored procedures for all database operations. The repository method is intentionally thin — it maps C# parameters to SQL parameters and maps result sets back to domain objects. |
| 9 | + |
| 10 | +### Stored procedures over inline SQL |
| 11 | + |
| 12 | +The default pattern is stored procedures for all Dapper database operations. Some exceptions exist where inline SQL is used — these are provided automatically by the repository base class and parent patterns, not written ad-hoc in individual repository methods. |
| 13 | + |
| 14 | +## Workflow |
| 15 | + |
| 16 | +1. **Define/update the stored procedure** in `src/Sql/dbo/Stored Procedures/` — use plain `CREATE PROCEDURE` (SSDT syntax) |
| 17 | +2. **Create a migration script** in `util/Migrator/DbScripts/` that deploys it — use `CREATE OR ALTER PROCEDURE` (idempotent) |
| 18 | +3. **Implement the repository method** in `src/Infrastructure/Dapper/Repositories/` using `DapperServiceProvider` to call the procedure |
| 19 | +4. **Write integration tests** using `[DatabaseData]` attribute |
| 20 | + |
| 21 | +The stored procedure is the source of truth for MSSQL query behavior. The Dapper repository method is thin — it maps parameters and results. |
| 22 | + |
| 23 | +### Stored procedure naming convention |
| 24 | + |
| 25 | +Procedures follow `{Entity}_{Action}` pattern: `User_Create`, `Cipher_ReadManyByUserId`, `Organization_DeleteById`. Tooling and code generation rely on this convention to map repository methods to their procedures. |
| 26 | + |
| 27 | +## Key Decisions That Trip Up AI Assistants |
| 28 | + |
| 29 | +### `CREATE OR ALTER` vs `CREATE PROCEDURE` — depends on file location |
| 30 | + |
| 31 | +Bitwarden maintains two copies of every stored procedure in different contexts with different toolchain constraints: |
| 32 | + |
| 33 | +| Context | Location | Required syntax | |
| 34 | +| ---------------------- | -------------------------------- | --------------------------- | |
| 35 | +| **SSDT schema source** | `src/Sql/dbo/Stored Procedures/` | `CREATE PROCEDURE` (plain) | |
| 36 | +| **Migration script** | `util/Migrator/DbScripts/` | `CREATE OR ALTER PROCEDURE` | |
| 37 | + |
| 38 | +**Why they differ:** |
| 39 | + |
| 40 | +- **SSDT projects** do not support `CREATE OR ALTER` — using it produces build errors. SSDT manages object lifecycle through its own deployment model, so each source file must contain a bare `CREATE PROCEDURE`. |
| 41 | +- **Migration scripts** must be idempotent because they may be re-run. `CREATE OR ALTER` works whether the procedure exists or not. Never use bare `CREATE PROCEDURE` in a migration. |
| 42 | + |
| 43 | +### SSDT table files require `GO` batch separators |
| 44 | + |
| 45 | +In `src/Sql/dbo/Tables/`, SSDT requires a `GO` batch separator between `CREATE TABLE` and any subsequent `CREATE INDEX` or `CREATE NONCLUSTERED INDEX` statements. |
| 46 | + |
| 47 | +```sql |
| 48 | +-- CORRECT — GO separates DDL statements for SSDT |
| 49 | +CREATE TABLE [dbo].[Example] ( |
| 50 | + [Id] UNIQUEIDENTIFIER NOT NULL, |
| 51 | + [Name] NVARCHAR(256) NOT NULL, |
| 52 | + CONSTRAINT [PK_Example] PRIMARY KEY CLUSTERED ([Id] ASC) |
| 53 | +) |
| 54 | +GO |
| 55 | + |
| 56 | +CREATE NONCLUSTERED INDEX [IX_Example_Name] |
| 57 | + ON [dbo].[Example] ([Name] ASC) |
| 58 | +GO |
| 59 | +``` |
| 60 | + |
| 61 | +### New parameters must be nullable with defaults |
| 62 | + |
| 63 | +When adding parameters to existing stored procedures, always use `@NewParam DATATYPE = NULL`. Existing callers don't pass the new parameter — without a default, they break. |
| 64 | + |
| 65 | +### NOT NULL columns: use inline defaults, not ALTER-UPDATE-ALTER |
| 66 | + |
| 67 | +Adding a NOT NULL column by first adding it nullable, updating all rows, then altering to NOT NULL causes a full table scan. Instead, use `ADD [Column] INT NOT NULL CONSTRAINT DF_Table_Column DEFAULT 0` — this is a metadata-only operation in SQL Server. **This is the single most common mistake AI assistants make with Bitwarden migrations.** |
| 68 | + |
| 69 | +### Never create indexes on large tables in migration scripts |
| 70 | + |
| 71 | +Creating indexes on `dbo.Cipher`, `dbo.OrganizationUser`, or other large tables in migration scripts can cause outages. Never specify `ONLINE = ON` in scripts — production handles this automatically, and the option fails on unsupported SQL Server editions. Large index operations belong in `DbScripts_manual`. |
| 72 | + |
| 73 | +### Use defaults only for numeric types |
| 74 | + |
| 75 | +Use defaults for `BIT`, `TINYINT`, `INT`, `BIGINT`. Never use defaults for `VARCHAR`, `NVARCHAR`, or MAX types. SQL Server handles these differently and defaults on strings create unexpected behavior with EF Core migrations. |
| 76 | + |
| 77 | +### Views require metadata refresh |
| 78 | + |
| 79 | +After modifying a table, any views that reference it have stale metadata. Call `sp_refreshview` on affected views. After altering views, call `sp_refreshsqlmodule` on dependent procedures. This is the most frequently forgotten step. |
| 80 | + |
| 81 | +### GUID columns use `UNIQUEIDENTIFIER` |
| 82 | + |
| 83 | +All entity IDs are `UNIQUEIDENTIFIER` populated by `CoreHelpers.GenerateComb()` in application code, not by SQL Server. Never use `NEWID()` or `NEWSEQUENTIALID()` in stored procedures. |
| 84 | + |
| 85 | +## EF Parity Requirement |
| 86 | + |
| 87 | +Every stored procedure's behavior must be exactly replicated in the EF Core implementation. When writing a new stored procedure, think about how the EF implementation will reproduce the same filtering, ordering, and side effects. If a stored procedure does something complex (e.g., conditional updates, multi-table operations), document the expected behavior clearly so the EF implementation can match it. |
| 88 | + |
| 89 | +## Critical Rules |
| 90 | + |
| 91 | +These are the most frequently violated conventions. Claude cannot fetch the linked docs at runtime, so these are inlined here: |
| 92 | + |
| 93 | +- **`SET NOCOUNT ON`** at the start of every stored procedure |
| 94 | +- **Parameter naming:** `@ParamName` in PascalCase, matching C# property names |
| 95 | +- **Migration scripts must be idempotent** — use `CREATE OR ALTER` in `util/Migrator/DbScripts/`; use plain `CREATE PROCEDURE` in SSDT source (`src/Sql/dbo/`) |
| 96 | +- **Constraint naming:** `PK_TableName`, `FK_Child_Parent`, `IX_Table_Column`, `DF_Table_Column` |
| 97 | +- **Stored procedure file naming:** one procedure per file, named `{Entity}_{Action}.sql` |
| 98 | + |
| 99 | +## Examples |
| 100 | + |
| 101 | +### Stored procedure creation — SSDT source vs migration script |
| 102 | + |
| 103 | +```sql |
| 104 | +-- SSDT source file: src/Sql/dbo/Stored Procedures/User_ReadById.sql |
| 105 | +-- Use plain CREATE PROCEDURE (SSDT does not support CREATE OR ALTER) |
| 106 | +CREATE PROCEDURE [dbo].[User_ReadById] |
| 107 | + @Id UNIQUEIDENTIFIER |
| 108 | +AS |
| 109 | +BEGIN |
| 110 | + SET NOCOUNT ON |
| 111 | + SELECT * FROM [dbo].[User] WHERE [Id] = @Id |
| 112 | +END |
| 113 | +``` |
| 114 | + |
| 115 | +```sql |
| 116 | +-- Migration script: util/Migrator/DbScripts/YYYY-MM-DD_00_AddUser_ReadById.sql |
| 117 | +-- Use CREATE OR ALTER for idempotency |
| 118 | +CREATE OR ALTER PROCEDURE [dbo].[User_ReadById] |
| 119 | + @Id UNIQUEIDENTIFIER |
| 120 | +AS |
| 121 | +BEGIN |
| 122 | + SET NOCOUNT ON |
| 123 | + SELECT * FROM [dbo].[User] WHERE [Id] = @Id |
| 124 | +END |
| 125 | +``` |
| 126 | + |
| 127 | +### Adding a NOT NULL column |
| 128 | + |
| 129 | +```sql |
| 130 | +-- CORRECT — metadata-only operation, no table scan |
| 131 | +ALTER TABLE [dbo].[Organization] |
| 132 | + ADD [UseCustomPermissions] BIT NOT NULL CONSTRAINT DF_Organization_UseCustomPermissions DEFAULT 0 |
| 133 | + |
| 134 | +-- WRONG — causes full table scan on large tables |
| 135 | +ALTER TABLE [dbo].[Organization] ADD [UseCustomPermissions] BIT NULL |
| 136 | +UPDATE [dbo].[Organization] SET [UseCustomPermissions] = 0 |
| 137 | +ALTER TABLE [dbo].[Organization] ALTER COLUMN [UseCustomPermissions] BIT NOT NULL |
| 138 | +``` |
| 139 | + |
| 140 | +### Adding parameters to existing procedures |
| 141 | + |
| 142 | +```sql |
| 143 | +-- CORRECT — existing callers won't break |
| 144 | +CREATE OR ALTER PROCEDURE [dbo].[Cipher_Create] |
| 145 | + @Id UNIQUEIDENTIFIER, |
| 146 | + @NewField NVARCHAR(MAX) = NULL -- default protects existing callers |
| 147 | + |
| 148 | +-- WRONG — breaks all existing callers immediately |
| 149 | +CREATE OR ALTER PROCEDURE [dbo].[Cipher_Create] |
| 150 | + @Id UNIQUEIDENTIFIER, |
| 151 | + @NewField NVARCHAR(MAX) -- no default = required parameter |
| 152 | +``` |
| 153 | + |
| 154 | +## Further Reading |
| 155 | + |
| 156 | +- [SQL code style](https://contributing.bitwarden.com/contributing/code-style/sql/) |
| 157 | +- [Database migrations (MSSQL)](https://contributing.bitwarden.com/contributing/database-migrations/mssql) |
0 commit comments