diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 3639ee3..dc9fb4e 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -24,7 +24,8 @@ "containerEnv": { "ASPNETCORE_ENVIRONMENT": "Development", "ASPNETCORE_URLS": "http://0.0.0.0:5165", - "DB_CONNECTION": "Server=sql,1433;Database=AppDb;User ID=sa;Password=LocalDev123!;Encrypt=False;TrustServerCertificate=True;" + "DB_CONNECTION": "Server=sql,1433;Database=AppDb;User ID=sa;Password=LocalDev123!;Encrypt=False;TrustServerCertificate=True;", + "DATA_DB_CONNECTION": "Server=sql,1433;Database=DataDb;User ID=sa;Password=LocalDev123!;Encrypt=False;TrustServerCertificate=True;" }, "postCreateCommand": "bash .devcontainer/setup.sh", "postStartCommand": "bash .devcontainer/wait-for-sql.sh && npm run start", diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 96f87ec..a674eb3 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -40,7 +40,7 @@ jobs: if find server.core/Migrations -type f -name '*.cs' -print0 \ | xargs -0 grep -nE "(\\[data\\]|\"data\"|'data'|(^|[^[:alnum:]_])data[.])"; then - echo "::error::EF migrations must not reference the data schema. The DACPAC owns [data]." + echo "::error::EF migrations must not reference the data schema. The data database is deployed from the DACPAC." failed=1 fi @@ -196,7 +196,7 @@ jobs: sql_database_name=$(az deployment group show \ --resource-group "$RESOURCE_GROUP" \ --name "$DEPLOYMENT_NAME" \ - --query "properties.outputs.sqlDatabaseName.value" \ + --query "properties.outputs.dataSqlDatabaseName.value" \ --output tsv) if [[ -z "$sql_server_name" ]]; then @@ -205,7 +205,7 @@ jobs: fi if [[ -z "$sql_database_name" ]]; then - echo "Deployment output sqlDatabaseName was empty." >&2 + echo "Deployment output dataSqlDatabaseName was empty." >&2 exit 1 fi @@ -220,7 +220,7 @@ jobs: exit 1 fi - echo "Resolved SQL database: $sql_server_fqdn/$sql_database_name" + echo "Resolved data SQL database: $sql_server_fqdn/$sql_database_name" echo "sql_server_fqdn=$sql_server_fqdn" >> "$GITHUB_OUTPUT" echo "sql_database_name=$sql_database_name" >> "$GITHUB_OUTPUT" @@ -256,7 +256,7 @@ jobs: /TargetUser:"$SQL_ADMIN_LOGIN" \ /TargetPassword:"$SQL_ADMIN_PASSWORD" \ /OutputPath:packages/scripts/prod-data-dacpac.sql \ - /p:DropObjectsNotInSource=False \ + /p:DropObjectsNotInSource=True \ /p:BlockOnPossibleDataLoss=True \ /p:CreateNewDatabase=False \ /p:ScriptDatabaseOptions=False diff --git a/.github/workflows/deploy-azure-appservice.yml b/.github/workflows/deploy-azure-appservice.yml index cbff5bd..c432af6 100644 --- a/.github/workflows/deploy-azure-appservice.yml +++ b/.github/workflows/deploy-azure-appservice.yml @@ -125,6 +125,12 @@ jobs: --query "properties.outputs.sqlDatabaseName.value" \ --output tsv) + data_sql_database_name=$(az deployment group show \ + --resource-group "$RESOURCE_GROUP" \ + --name "$DEPLOYMENT_NAME" \ + --query "properties.outputs.dataSqlDatabaseName.value" \ + --output tsv) + if [[ -z "$web_app_name" ]]; then echo "Deployment output webAppName was empty." >&2 exit 1 @@ -140,6 +146,11 @@ jobs: exit 1 fi + if [[ -z "$data_sql_database_name" ]]; then + echo "Deployment output dataSqlDatabaseName was empty." >&2 + exit 1 + fi + sql_server_fqdn=$(az sql server show \ --resource-group "$RESOURCE_GROUP" \ --name "$sql_server_name" \ @@ -152,10 +163,12 @@ jobs: fi echo "Resolved web app: $web_app_name" - echo "Resolved SQL database: $sql_server_fqdn/$sql_database_name" + echo "Resolved app SQL database: $sql_server_fqdn/$sql_database_name" + echo "Resolved data SQL database: $sql_server_fqdn/$data_sql_database_name" echo "web_app_name=$web_app_name" >> "$GITHUB_OUTPUT" echo "sql_server_fqdn=$sql_server_fqdn" >> "$GITHUB_OUTPUT" echo "sql_database_name=$sql_database_name" >> "$GITHUB_OUTPUT" + echo "data_sql_database_name=$data_sql_database_name" >> "$GITHUB_OUTPUT" - name: Install SQLPackage run: | @@ -176,6 +189,8 @@ jobs: env: ENVIRONMENT_NAME: ${{ env.ENVIRONMENT }} WEB_APP_NAME: ${{ steps.resolve.outputs.web_app_name }} + SQL_SERVER_FQDN: ${{ steps.resolve.outputs.sql_server_fqdn }} + DATA_SQL_DATABASE_NAME: ${{ steps.resolve.outputs.data_sql_database_name }} SMTP_HOST: ${{ vars.Smtp__Host }} SMTP_PORT: ${{ vars.Smtp__Port }} SMTP_TIMEOUT: ${{ vars.Smtp__Timeout }} @@ -210,7 +225,10 @@ jobs: settings+=("$name=$value") } + data_db_connection="Server=tcp:$SQL_SERVER_FQDN,1433;Initial Catalog=$DATA_SQL_DATABASE_NAME;Persist Security Info=False;User ID=$SQL_ADMIN_LOGIN;Password=$SQL_ADMIN_PASSWORD;MultipleActiveResultSets=False;Encrypt=True;TrustServerCertificate=False;Connection Timeout=30;" + append_setting "Notification__BaseUrl" "$NOTIFICATION_BASE_URL" + append_setting "DATA_DB_CONNECTION" "$data_db_connection" append_setting "Smtp__Host" "$SMTP_HOST" append_setting "Smtp__Port" "$SMTP_PORT" append_setting "Smtp__Timeout" "$SMTP_TIMEOUT" @@ -235,7 +253,7 @@ jobs: - name: Publish data DACPAC env: SQL_SERVER_FQDN: ${{ steps.resolve.outputs.sql_server_fqdn }} - SQL_DATABASE_NAME: ${{ steps.resolve.outputs.sql_database_name }} + DATA_SQL_DATABASE_NAME: ${{ steps.resolve.outputs.data_sql_database_name }} run: | set -euo pipefail @@ -248,10 +266,10 @@ jobs: /Action:Publish \ /SourceFile:packages/data/data.dacpac \ /TargetServerName:"$SQL_SERVER_FQDN" \ - /TargetDatabaseName:"$SQL_DATABASE_NAME" \ + /TargetDatabaseName:"$DATA_SQL_DATABASE_NAME" \ /TargetUser:"$SQL_ADMIN_LOGIN" \ /TargetPassword:"$SQL_ADMIN_PASSWORD" \ - /p:DropObjectsNotInSource=False \ + /p:DropObjectsNotInSource=True \ /p:BlockOnPossibleDataLoss=True \ /p:CreateNewDatabase=False \ /p:ScriptDatabaseOptions=False diff --git a/README.md b/README.md index 34c2ff4..d010f22 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ AD419 is a full-stack web application built with a .NET 10 backend and a React/V - **Frontend**: React 19 with Vite, TypeScript, and TanStack Router/Query/Table - **Authentication**: OIDC with Microsoft Entra ID (Azure AD) - **Styling**: Tailwind CSS -- **Database**: SQL Server, with EF Core migrations for application tables and a SQL project/DACPAC for reference/import data tables +- **Database**: SQL Server, with EF Core migrations for application tables and a separate SQL database deployed from a SQL project/DACPAC for reference/import data tables - **Development**: Hot reload for both frontend and backend - **Development Integration**: ASP.NET Core `SpaProxy` launches Vite for Visual Studio users, while Vite proxies API and auth routes back to ASP.NET Core during development @@ -92,12 +92,12 @@ In development, the frontend runs from **http://localhost:5173** and proxies bac ### Database configuration -The backend requires a SQL Server connection string. +The backend requires SQL Server connection strings for the app database and the data database. -- Outside DevContainer, the default development connection points to the SQL Server container published on `localhost:14333`. -- Inside DevContainer, `devcontainer.json` overrides `DB_CONNECTION` to use the internal Docker hostname `sql:1433`. +- Outside DevContainer, the default development connections point to the SQL Server container published on `localhost:14333`. EF uses `AppDb`; the data DACPAC and imports use `DataDb`. +- Inside DevContainer, `devcontainer.json` overrides `DB_CONNECTION` and `DATA_DB_CONNECTION` to use the internal Docker hostname `sql:1433`. -When you want to specify your own DB connection, provide it by setting the `DB_CONNECTION` environment variable (for example in a `.env` file) or by updating `ConnectionStrings:DefaultConnection` in `appsettings.*.json` (`.env` is recommended) +When you want to specify your own DB connections, provide them by setting the `DB_CONNECTION` and `DATA_DB_CONNECTION` environment variables (for example in a `.env` file) or by updating `ConnectionStrings:DefaultConnection` and `ConnectionStrings:DataConnection` in `appsettings.*.json` (`.env` is recommended). To run only the database outside DevContainer: @@ -114,10 +114,10 @@ Useful companion commands: ### Database schema and deployment -AD419 has two database schema-management paths with different ownership boundaries: +AD419 has two database-management paths with different ownership boundaries: -- EF Core migrations in `server.core/Migrations/` manage the application schema, currently the `[app]` schema and application tables such as `[app].[Users]`. -- The SQL project in `database/data/data.sqlproj` builds a DACPAC for the `[data]` schema and data-import/reference tables such as `[data].[AllProjects]`, `[data].[ActiveProjects]`, and `[data].[AssistanceListingNumbers]`. +- EF Core migrations in `server.core/Migrations/` manage the application database, currently the `[app]` schema and application tables such as `[app].[Users]`. +- The SQL project in `database/data/data.sqlproj` builds a DACPAC for a separate data database containing the `[data]` schema and data-import/reference tables such as `[data].[AllProjects]`, `[data].[ActiveProjects]`, and `[data].[AssistanceListingNumbers]`. The backend runs EF Core migrations at startup through `DbInitializer` and `AppDbContext.Database.MigrateAsync()`. The EF migrations assembly is `server.core`, and the EF migrations history table is stored as `[app].[__EFMigrationsHistory]`. @@ -143,9 +143,9 @@ To publish the data DACPAC to the local SQL Server container: ./database/data/publish-local.sh ``` -`publish-local.sh` uses `BUILD_CONFIGURATION` to choose `Debug` or `Release`, `SQLPACKAGE` to locate the `sqlpackage` executable, and `DB_CONNECTION` for the target connection string. If `DB_CONNECTION` is not set, it targets the local container database at `localhost:14333`. +`publish-local.sh` uses `BUILD_CONFIGURATION` to choose `Debug` or `Release`, `SQLPACKAGE` to locate the `sqlpackage` executable, and `DATA_DB_CONNECTION` for the target connection string. If `DATA_DB_CONNECTION` is not set, it targets the local container `DataDb` database at `localhost:14333`. Local publish creates `DataDb` by default; set `CREATE_NEW_DATABASE=False` to require the database to already exist. -In GitHub Actions, `ci-cd.yml` builds the solution, verifies schema ownership boundaries, uploads the web app package and `data.dacpac`, and `deploy-azure-appservice.yml` publishes the data DACPAC before deploying the web app. The DACPAC publish does not broadly drop objects that are missing from the data project; removed `[data]` objects that must be dropped are handled explicitly in SQL project deployment scripts. Publish blocks possible data loss, does not create the database, and does not script database options. +In GitHub Actions, `ci-cd.yml` builds the solution, verifies schema ownership boundaries, uploads the web app package and `data.dacpac`, and `deploy-azure-appservice.yml` publishes the data DACPAC to the separate data database before deploying the web app. The DACPAC publish does not broadly drop objects that are missing from the data project; removed `[data]` objects that must be dropped are handled explicitly in SQL project deployment scripts. Azure infrastructure creates both databases on the same SQL server, so workflow publish blocks possible data loss, does not create the database, and does not script database options. For production deployments, the workflow first generates and uploads a `prod-data-dacpac-script` artifact with SQLPackage `/Action:Script`. Review that script before approving the gated production publish job. diff --git a/database/data/Scripts/Script.PreDeployment.sql b/database/data/Scripts/Script.PreDeployment.sql index 3411292..38c8588 100644 --- a/database/data/Scripts/Script.PreDeployment.sql +++ b/database/data/Scripts/Script.PreDeployment.sql @@ -1,22 +1,3 @@ /* Pre-deployment cleanup for objects intentionally removed from the data DACPAC. - -Keep drops here scoped to the [data] schema so SQLPackage does not need broad -DropObjectsNotInSource behavior across the whole database. */ - -IF EXISTS -( - SELECT 1 - FROM sys.key_constraints kc - JOIN sys.tables t ON t.object_id = kc.parent_object_id - JOIN sys.schemas s ON s.schema_id = t.schema_id - WHERE s.name = N'data' - AND t.name = N'AllProjects' - AND kc.name = N'UQ_AllProjects_ProjectNumber' -) -BEGIN - ALTER TABLE [data].[AllProjects] - DROP CONSTRAINT [UQ_AllProjects_ProjectNumber]; -END; -GO diff --git a/database/data/publish-local.sh b/database/data/publish-local.sh index e6487a7..390df03 100755 --- a/database/data/publish-local.sh +++ b/database/data/publish-local.sh @@ -6,7 +6,9 @@ configuration="${BUILD_CONFIGURATION:-Debug}" project="$script_dir/data.sqlproj" dacpac="$script_dir/bin/$configuration/data.dacpac" sqlpackage="${SQLPACKAGE:-/usr/local/sqlpackage/sqlpackage}" -connection_string="${DB_CONNECTION:-Server=localhost,14333;Database=AppDb;User ID=sa;Password=LocalDev123!;Encrypt=False;TrustServerCertificate=True;}" +connection_string="${DATA_DB_CONNECTION:-Server=localhost,14333;Database=DataDb;User ID=sa;Password=LocalDev123!;Encrypt=False;TrustServerCertificate=True;}" +# Set CREATE_NEW_DATABASE=True only when you explicitly want sqlpackage to drop and recreate DataDb. +create_new_database="${CREATE_NEW_DATABASE:-False}" if [[ ! -x "$sqlpackage" ]]; then echo "SQLPackage was not found or is not executable at '$sqlpackage'." >&2 @@ -20,7 +22,7 @@ dotnet build "$project" -c "$configuration" /p:DSP=Microsoft.Data.Tools.Schema.S /Action:Publish \ /SourceFile:"$dacpac" \ "/TargetConnectionString:$connection_string" \ - /p:DropObjectsNotInSource=False \ + /p:DropObjectsNotInSource=True \ /p:BlockOnPossibleDataLoss=True \ - /p:CreateNewDatabase=False \ + /p:CreateNewDatabase="$create_new_database" \ /p:ScriptDatabaseOptions=False diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2a6415b..17528e5 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -8,7 +8,7 @@ AD419 uses: - Vite on port `5173` for the React frontend during development - ASP.NET Core `SpaProxy` so Visual Studio can launch the frontend without a separate `.esproj` - Vite proxy rules so frontend requests to `/api`, `/login`, `/signin-oidc`, and `/health` are forwarded to ASP.NET Core -- SQL Server schema management split between EF Core migrations for application-owned tables and a SQL project/DACPAC for data-import/reference tables +- SQL Server database management split between EF Core migrations for application-owned tables and a SQL project/DACPAC for data-import/reference tables In production, ASP.NET Core serves the built frontend from `server/wwwroot`. @@ -56,11 +56,11 @@ Browser → :5165 (ASP.NET Core) ## Database Schema Ownership -AD419 keeps the application schema and import/reference data schema separate. +AD419 keeps the application database and import/reference data database separate. ### EF Core application schema -EF Core owns the `[app]` schema. `server.core/Data/AppDbContext.cs` sets the default schema to `[app]`, and migrations live in `server.core/Migrations/`. +EF Core owns the application database and its `[app]` schema. `server.core/Data/AppDbContext.cs` sets the default schema to `[app]`, and migrations live in `server.core/Migrations/`. At startup, `server/Program.cs` resolves `IDbInitializer`, which runs `AppDbContext.Database.MigrateAsync()`. This means application deployments apply pending EF migrations when the web app starts. EF's migrations history table is configured as `[app].[__EFMigrationsHistory]`. @@ -74,9 +74,9 @@ Local helpers: Only create EF migrations for model changes that affect the application-owned schema. Shared migrations should not be edited in place. -### Data DACPAC schema +### Data DACPAC database -The SQL project at `database/data/data.sqlproj` owns the `[data]` schema and data-import/reference tables. It currently includes: +The SQL project at `database/data/data.sqlproj` owns the separate data database, including its `[data]` schema and data-import/reference tables. It currently includes: - `database/data/Schemas/data.sql` - `database/data/Tables/AllProjects.sql` @@ -90,16 +90,16 @@ The SQL project builds `database/data/bin//data.dacpac`. For loca ./database/data/publish-local.sh ``` -The script uses `SQLPACKAGE` when set, otherwise `/usr/local/sqlpackage/sqlpackage`. It uses `DB_CONNECTION` when set, otherwise the local development SQL Server connection string for `localhost:14333`. +The script uses `SQLPACKAGE` when set, otherwise `/usr/local/sqlpackage/sqlpackage`. It uses `DATA_DB_CONNECTION` when set, otherwise the local development SQL Server connection string for `DataDb` on `localhost:14333`. Local publish creates `DataDb` by default; set `CREATE_NEW_DATABASE=False` to require it to already exist. CI builds the SQL project through `app.sln`, uploads `data.dacpac` as a `data-dacpac` artifact, and the reusable Azure deployment workflow publishes it before deploying the web app. DACPAC publish settings are: -- `DropObjectsNotInSource=False` +- `DropObjectsNotInSource=True` - `BlockOnPossibleDataLoss=True` - `CreateNewDatabase=False` - `ScriptDatabaseOptions=False` -This lets the data DACPAC add and update objects that are owned by the data SQL project without dropping unrelated objects in other schemas. Removed `[data]` objects that must be dropped are handled explicitly in SQL project deployment scripts. +Azure infrastructure creates both the EF application database and the data database on the same SQL server. The data DACPAC is published only to the data database, which avoids DACPAC's database-wide deployment scope colliding with EF-owned objects. Removed `[data]` objects that must be dropped are handled explicitly in SQL project deployment scripts. The CI workflow also enforces the schema boundary by failing if EF migrations reference the `data` schema or the data SQL project references the `app` schema. Production deploys add one review step before publishing: a separate job generates a `prod-data-dacpac-script` artifact with SQLPackage `/Action:Script`, then the production publish runs behind the `deploy_prod` workflow input and `prod` GitHub Environment gate. @@ -165,7 +165,7 @@ Responsibilities: Responsibilities: - Builds the `data.dacpac` -- Defines the `[data]` schema and data/import tables +- Defines the data database's `[data]` schema and data/import tables - Includes the post-deployment script hook ### `.github/workflows/ci-cd.yml` @@ -183,7 +183,7 @@ Responsibilities: Responsibilities: - Deploys or updates Azure infrastructure -- Resolves the target App Service and SQL Database from deployment outputs +- Resolves the target App Service, application SQL database, and data SQL database from deployment outputs - Publishes the data DACPAC with SQLPackage - Deploys the web app package diff --git a/infrastructure/azure/README.md b/infrastructure/azure/README.md index 8545a26..b2c5640 100644 --- a/infrastructure/azure/README.md +++ b/infrastructure/azure/README.md @@ -1,6 +1,6 @@ # Azure infrastructure -AD419 deploys to Linux Azure App Service with Azure SQL Database and workspace-based Application Insights. GitHub Actions now owns CI/CD. +AD419 deploys to Linux Azure App Service with Azure SQL databases and workspace-based Application Insights. GitHub Actions now owns CI/CD. ## GitHub Actions @@ -62,7 +62,7 @@ Suggested values: | `test` | same as `TEST_AZURE_SUBSCRIPTION_ID` | `rg-ad419-test` | `test` | `ad419-test` | | `prod` | same as `PROD_AZURE_SUBSCRIPTION_ID` | `rg-ad419-prod` | `prod` | `ad419-prod` | -The Bicep deployment sets `DB_CONNECTION`, `WEBSITE_RUN_FROM_PACKAGE`, Application Insights settings, and a default `Notification__BaseUrl`. The workflow can override notification, SMTP, and OTLP settings from GitHub Environment configuration. +The Bicep deployment creates an application database and a separate data database on the same SQL server. It sets `DB_CONNECTION`, `DATA_DB_CONNECTION`, `WEBSITE_RUN_FROM_PACKAGE`, Application Insights settings, and a default `Notification__BaseUrl`. The workflow can override notification, SMTP, and OTLP settings from GitHub Environment configuration. ## OIDC bootstrap diff --git a/infrastructure/azure/main.bicep b/infrastructure/azure/main.bicep index b4fc2b1..20fc7f7 100644 --- a/infrastructure/azure/main.bicep +++ b/infrastructure/azure/main.bicep @@ -24,9 +24,12 @@ param sqlAdminLogin string @description('SQL admin password for SQL authentication.') param sqlAdminPassword string -@description('SQL database name.') +@description('Application SQL database name.') param sqlDatabaseName string = appName +@description('Data SQL database name.') +param dataSqlDatabaseName string = '${appName}-data' + @description('Additional resource tags to apply.') param tags object = {} @@ -91,7 +94,8 @@ module sql 'modules/sql.bicep' = if (deploymentGuardPassed) { tags: resourceTags adminLogin: sqlAdminLogin adminPassword: sqlAdminPassword - databaseName: sqlDatabaseName + appDatabaseName: sqlDatabaseName + dataDatabaseName: dataSqlDatabaseName skuName: sqlSkuName skuTier: sqlSkuTier } @@ -100,6 +104,7 @@ module sql 'modules/sql.bicep' = if (deploymentGuardPassed) { var sqlServerHostnameSuffix = environment().suffixes.sqlServerHostname var sqlServerFqdn = '${sqlServerName}${startsWith(sqlServerHostnameSuffix, '.') ? '' : '.'}${sqlServerHostnameSuffix}' var sqlConnectionString = 'Server=tcp:${sqlServerFqdn},1433;Initial Catalog=${sqlDatabaseName};Persist Security Info=False;User ID=${sqlAdminLogin};Password=${sqlAdminPassword};MultipleActiveResultSets=False;Encrypt=True;TrustServerCertificate=False;Connection Timeout=30;' +var dataSqlConnectionString = 'Server=tcp:${sqlServerFqdn},1433;Initial Catalog=${dataSqlDatabaseName};Persist Security Info=False;User ID=${sqlAdminLogin};Password=${sqlAdminPassword};MultipleActiveResultSets=False;Encrypt=True;TrustServerCertificate=False;Connection Timeout=30;' module compute 'modules/compute.bicep' = if (deploymentGuardPassed) { name: 'compute' @@ -111,11 +116,15 @@ module compute 'modules/compute.bicep' = if (deploymentGuardPassed) { webSkuName: webSkuName webSkuTier: webSkuTier sqlConnectionString: sqlConnectionString + dataSqlConnectionString: dataSqlConnectionString environmentName: env appInsightsConnectionString: appInsights!.properties.ConnectionString appInsightsInstrumentationKey: appInsights!.properties.InstrumentationKey notificationBaseUrl: empty(notificationBaseUrl) ? 'https://${webAppName}.azurewebsites.net' : notificationBaseUrl } + dependsOn: [ + sql + ] } output appServiceDefaultHostName string = deploymentGuardPassed ? compute!.outputs.defaultHostName : '' @@ -124,6 +133,7 @@ output appInsightsName string = deploymentGuardPassed ? appInsights!.name : '' output appInsightsConnectionString string = deploymentGuardPassed ? appInsights!.properties.ConnectionString : '' output logAnalyticsWorkspaceName string = deploymentGuardPassed ? logAnalyticsWorkspace!.name : '' output sqlDatabaseName string = sqlDatabaseName +output dataSqlDatabaseName string = dataSqlDatabaseName output sqlServerName string = deploymentGuardPassed ? sql!.outputs.serverName : '' output webAppName string = deploymentGuardPassed ? compute!.outputs.webAppName : '' output deploymentGuardPassed bool = deploymentGuardPassed diff --git a/infrastructure/azure/main.json b/infrastructure/azure/main.json index 196d009..9695b1b 100644 --- a/infrastructure/azure/main.json +++ b/infrastructure/azure/main.json @@ -58,7 +58,14 @@ "type": "string", "defaultValue": "[parameters('appName')]", "metadata": { - "description": "SQL database name." + "description": "Application SQL database name." + } + }, + "dataSqlDatabaseName": { + "type": "string", + "defaultValue": "[format('{0}-data', parameters('appName'))]", + "metadata": { + "description": "Data SQL database name." } }, "tags": { @@ -104,7 +111,8 @@ "resourceTags": "[union(parameters('tags'), createObject('environment', parameters('env'), 'application', parameters('appName')))]", "sqlServerHostnameSuffix": "[environment().suffixes.sqlServerHostname]", "sqlServerFqdn": "[format('{0}{1}{2}', variables('sqlServerName'), if(startsWith(variables('sqlServerHostnameSuffix'), '.'), '', '.'), variables('sqlServerHostnameSuffix'))]", - "sqlConnectionString": "[format('Server=tcp:{0},1433;Initial Catalog={1};Persist Security Info=False;User ID={2};Password={3};MultipleActiveResultSets=False;Encrypt=True;TrustServerCertificate=False;Connection Timeout=30;', variables('sqlServerFqdn'), parameters('sqlDatabaseName'), parameters('sqlAdminLogin'), parameters('sqlAdminPassword'))]" + "sqlConnectionString": "[format('Server=tcp:{0},1433;Initial Catalog={1};Persist Security Info=False;User ID={2};Password={3};MultipleActiveResultSets=False;Encrypt=True;TrustServerCertificate=False;Connection Timeout=30;', variables('sqlServerFqdn'), parameters('sqlDatabaseName'), parameters('sqlAdminLogin'), parameters('sqlAdminPassword'))]", + "dataSqlConnectionString": "[format('Server=tcp:{0},1433;Initial Catalog={1};Persist Security Info=False;User ID={2};Password={3};MultipleActiveResultSets=False;Encrypt=True;TrustServerCertificate=False;Connection Timeout=30;', variables('sqlServerFqdn'), parameters('dataSqlDatabaseName'), parameters('sqlAdminLogin'), parameters('sqlAdminPassword'))]" }, "resources": [ { @@ -163,9 +171,12 @@ "adminPassword": { "value": "[parameters('sqlAdminPassword')]" }, - "databaseName": { + "appDatabaseName": { "value": "[parameters('sqlDatabaseName')]" }, + "dataDatabaseName": { + "value": "[parameters('dataSqlDatabaseName')]" + }, "skuName": { "value": "[variables('sqlSkuName')]" }, @@ -214,10 +225,16 @@ "description": "SQL admin password for SQL authentication." } }, - "databaseName": { + "appDatabaseName": { + "type": "string", + "metadata": { + "description": "Application SQL database name." + } + }, + "dataDatabaseName": { "type": "string", "metadata": { - "description": "SQL database name." + "description": "Data SQL database name." } }, "skuName": { @@ -272,7 +289,23 @@ { "type": "Microsoft.Sql/servers/databases", "apiVersion": "2023-08-01", - "name": "[format('{0}/{1}', parameters('name'), parameters('databaseName'))]", + "name": "[format('{0}/{1}', parameters('name'), parameters('appDatabaseName'))]", + "location": "[parameters('location')]", + "sku": { + "name": "[parameters('skuName')]", + "tier": "[parameters('skuTier')]" + }, + "properties": { + "collation": "SQL_Latin1_General_CP1_CI_AS" + }, + "dependsOn": [ + "[resourceId('Microsoft.Sql/servers', parameters('name'))]" + ] + }, + { + "type": "Microsoft.Sql/servers/databases", + "apiVersion": "2023-08-01", + "name": "[format('{0}/{1}', parameters('name'), parameters('dataDatabaseName'))]", "location": "[parameters('location')]", "sku": { "name": "[parameters('skuName')]", @@ -287,9 +320,13 @@ } ], "outputs": { - "databaseName": { + "appDatabaseName": { + "type": "string", + "value": "[parameters('appDatabaseName')]" + }, + "dataDatabaseName": { "type": "string", - "value": "[parameters('databaseName')]" + "value": "[parameters('dataDatabaseName')]" }, "serverName": { "type": "string", @@ -331,6 +368,9 @@ "sqlConnectionString": { "value": "[variables('sqlConnectionString')]" }, + "dataSqlConnectionString": { + "value": "[variables('dataSqlConnectionString')]" + }, "environmentName": { "value": "[parameters('env')]" }, @@ -404,6 +444,12 @@ "description": "SQL connection string." } }, + "dataSqlConnectionString": { + "type": "securestring", + "metadata": { + "description": "Data SQL database connection string." + } + }, "environmentName": { "type": "string", "metadata": { @@ -476,6 +522,10 @@ "name": "DB_CONNECTION", "value": "[parameters('sqlConnectionString')]" }, + { + "name": "DATA_DB_CONNECTION", + "value": "[parameters('dataSqlConnectionString')]" + }, { "name": "Notification__BaseUrl", "value": "[parameters('notificationBaseUrl')]" @@ -521,7 +571,8 @@ } }, "dependsOn": [ - "[resourceId('Microsoft.Insights/components', variables('appInsightsName'))]" + "[resourceId('Microsoft.Insights/components', variables('appInsightsName'))]", + "[resourceId('Microsoft.Resources/deployments', 'sql')]" ] } ], @@ -550,6 +601,10 @@ "type": "string", "value": "[parameters('sqlDatabaseName')]" }, + "dataSqlDatabaseName": { + "type": "string", + "value": "[parameters('dataSqlDatabaseName')]" + }, "sqlServerName": { "type": "string", "value": "[if(variables('deploymentGuardPassed'), reference(resourceId('Microsoft.Resources/deployments', 'sql'), '2025-04-01').outputs.serverName.value, '')]" @@ -563,4 +618,4 @@ "value": "[variables('deploymentGuardPassed')]" } } -} \ No newline at end of file +} diff --git a/infrastructure/azure/modules/compute.bicep b/infrastructure/azure/modules/compute.bicep index 8561848..2e602f7 100644 --- a/infrastructure/azure/modules/compute.bicep +++ b/infrastructure/azure/modules/compute.bicep @@ -23,6 +23,10 @@ param linuxFxVersion string = 'DOTNETCORE|10.0' @description('SQL connection string.') param sqlConnectionString string +@secure() +@description('Data SQL database connection string.') +param dataSqlConnectionString string + @description('Environment name for app settings.') param environmentName string @@ -78,6 +82,10 @@ resource webApp 'Microsoft.Web/sites@2023-12-01' = { name: 'DB_CONNECTION' value: sqlConnectionString } + { + name: 'DATA_DB_CONNECTION' + value: dataSqlConnectionString + } { name: 'Notification__BaseUrl' value: notificationBaseUrl diff --git a/infrastructure/azure/modules/sql.bicep b/infrastructure/azure/modules/sql.bicep index 701870a..c178458 100644 --- a/infrastructure/azure/modules/sql.bicep +++ b/infrastructure/azure/modules/sql.bicep @@ -14,8 +14,11 @@ param adminLogin string @description('SQL admin password for SQL authentication.') param adminPassword string -@description('SQL database name.') -param databaseName string +@description('Application SQL database name.') +param appDatabaseName string + +@description('Data SQL database name.') +param dataDatabaseName string @description('SQL database SKU name.') param skuName string = 'S0' @@ -47,8 +50,21 @@ resource allowAzureServicesFirewallRule 'Microsoft.Sql/servers/firewallRules@202 } } -resource database 'Microsoft.Sql/servers/databases@2023-08-01' = { - name: databaseName +resource appDatabase 'Microsoft.Sql/servers/databases@2023-08-01' = { + name: appDatabaseName + parent: sqlServer + location: location + sku: { + name: skuName + tier: skuTier + } + properties: { + collation: 'SQL_Latin1_General_CP1_CI_AS' + } +} + +resource dataDatabase 'Microsoft.Sql/servers/databases@2023-08-01' = { + name: dataDatabaseName parent: sqlServer location: location sku: { @@ -60,5 +76,6 @@ resource database 'Microsoft.Sql/servers/databases@2023-08-01' = { } } -output databaseName string = database.name +output appDatabaseName string = appDatabase.name +output dataDatabaseName string = dataDatabase.name output serverName string = sqlServer.name diff --git a/server.core/Data/DataDbConnection.cs b/server.core/Data/DataDbConnection.cs new file mode 100644 index 0000000..e47d1ea --- /dev/null +++ b/server.core/Data/DataDbConnection.cs @@ -0,0 +1,31 @@ +using Microsoft.Extensions.Configuration; + +namespace Server.Core.Data; + +public static class DataDbConnection +{ + public const string EnvironmentVariableName = "DATA_DB_CONNECTION"; + public const string ConnectionStringName = "DataConnection"; + + public static string Resolve(IConfiguration configuration, string? fallbackConnectionString) + { + var connectionString = Coalesce( + configuration[EnvironmentVariableName], + configuration.GetConnectionString(ConnectionStringName), + fallbackConnectionString); + + if (string.IsNullOrWhiteSpace(connectionString)) + { + throw new InvalidOperationException( + $"No data database connection string configured. Set the {EnvironmentVariableName} environment variable " + + $"or configure ConnectionStrings:{ConnectionStringName}."); + } + + return connectionString; + } + + private static string? Coalesce(params string?[] values) + { + return values.FirstOrDefault(value => !string.IsNullOrWhiteSpace(value)); + } +} diff --git a/server.core/Import/PgmProjectsImportService.cs b/server.core/Import/PgmProjectsImportService.cs index 5dde4ca..4d0db1d 100644 --- a/server.core/Import/PgmProjectsImportService.cs +++ b/server.core/Import/PgmProjectsImportService.cs @@ -84,8 +84,9 @@ public async Task ImportAsync(DateOnly reportDate, Canc $"or configure ConnectionStrings:{ConnectionStringName}."); } - var destinationConnectionString = _dbContext.Database.GetConnectionString() - ?? throw new InvalidOperationException("The application database connection string is not available."); + var destinationConnectionString = DataDbConnection.Resolve( + _configuration, + _dbContext.Database.GetConnectionString()); _logger.LogInformation("Importing PGM projects for report date {ReportDate}", reportDate); diff --git a/server/.env.example b/server/.env.example index 7923d83..e0fe136 100644 --- a/server/.env.example +++ b/server/.env.example @@ -5,6 +5,7 @@ OTEL_SERVICE_NAME="" # database DB_CONNECTION="Server=localhost,14333;Database=AppDb;User ID=sa;Password=LocalDev123!;Encrypt=False;TrustServerCertificate=True;" +DATA_DB_CONNECTION="Server=localhost,14333;Database=DataDb;User ID=sa;Password=LocalDev123!;Encrypt=False;TrustServerCertificate=True;" # smtp delivery Smtp__Host="" diff --git a/server/Import/FlatFileImportService.cs b/server/Import/FlatFileImportService.cs index 4572283..ce344f3 100644 --- a/server/Import/FlatFileImportService.cs +++ b/server/Import/FlatFileImportService.cs @@ -9,6 +9,7 @@ using DocumentFormat.OpenXml.Packaging; using Microsoft.Data.SqlClient; using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Configuration; using Server.Authorization; using Server.Core.Data; using Server.Core.Domain; @@ -34,6 +35,7 @@ public sealed record ImportValidationFailed(ImportValidationResponse Response) : public sealed class FlatFileImportService( AppDbContext dbContext, IFlatFileImportRegistry registry, + IConfiguration configuration, ILogger logger) : IFlatFileImportService { private const string TempTableName = "#FlatFileImportRows"; @@ -763,72 +765,57 @@ private async Task ReplaceTargetTableAsync( IReadOnlyList rows, CancellationToken cancellationToken) { - var connectionString = dbContext.Database.GetConnectionString(); - if (string.IsNullOrWhiteSpace(connectionString)) - { - throw new InvalidOperationException("No database connection string is configured."); - } + var connectionString = DataDbConnection.Resolve( + configuration, + dbContext.Database.GetConnectionString()); await using var connection = new SqlConnection(connectionString); await connection.OpenAsync(cancellationToken); + await using var transaction = (SqlTransaction)await connection.BeginTransactionAsync(cancellationToken); - await DropTempTableIfExistsAsync(connection, cancellationToken); - try - { - await using var transaction = (SqlTransaction)await connection.BeginTransactionAsync(cancellationToken); + await DropTempTableIfExistsAsync(connection, transaction, cancellationToken); - await connection.ExecuteAsync( - CreateTempTableSql(definition), - transaction: transaction); - - var table = CreateDataTable(definition, rows); - using (var bulkCopy = new SqlBulkCopy(connection, SqlBulkCopyOptions.CheckConstraints, transaction)) - { - bulkCopy.DestinationTableName = TempTableName; - bulkCopy.BatchSize = Math.Max(rows.Count, 1); + await connection.ExecuteAsync(new CommandDefinition( + CreateTempTableSql(definition), + transaction: transaction, + cancellationToken: cancellationToken)); - bulkCopy.ColumnMappings.Add("ImportRowNumber", "ImportRowNumber"); - foreach (var column in definition.Columns) - { - bulkCopy.ColumnMappings.Add(column.TargetColumn, column.TargetColumn); - } + var table = CreateDataTable(definition, rows); + using (var bulkCopy = new SqlBulkCopy(connection, SqlBulkCopyOptions.CheckConstraints, transaction)) + { + bulkCopy.DestinationTableName = TempTableName; + bulkCopy.BatchSize = Math.Max(rows.Count, 1); - await bulkCopy.WriteToServerAsync(table, cancellationToken); + bulkCopy.ColumnMappings.Add("ImportRowNumber", "ImportRowNumber"); + foreach (var column in definition.Columns) + { + bulkCopy.ColumnMappings.Add(column.TargetColumn, column.TargetColumn); } - await RunStagingValidationAsync(connection, transaction, definition, rows, cancellationToken); + await bulkCopy.WriteToServerAsync(table, cancellationToken); + } - await connection.ExecuteAsync( - ReplaceTableSql(definition), - transaction: transaction); + await RunStagingValidationAsync(connection, transaction, definition, rows, cancellationToken); - await transaction.CommitAsync(cancellationToken); - } - finally - { - await DropTempTableIfExistsBestEffortAsync(connection); - } + await connection.ExecuteAsync(new CommandDefinition( + ReplaceTableSql(definition), + transaction: transaction, + cancellationToken: cancellationToken)); + + await transaction.CommitAsync(cancellationToken); } - private static Task DropTempTableIfExistsAsync(SqlConnection connection, CancellationToken cancellationToken) + private static Task DropTempTableIfExistsAsync( + SqlConnection connection, + SqlTransaction transaction, + CancellationToken cancellationToken) { return connection.ExecuteAsync(new CommandDefinition( $"DROP TABLE IF EXISTS {TempTableName};", + transaction: transaction, cancellationToken: cancellationToken)); } - private async Task DropTempTableIfExistsBestEffortAsync(SqlConnection connection) - { - try - { - await DropTempTableIfExistsAsync(connection, CancellationToken.None); - } - catch (Exception ex) when (ex is SqlException or InvalidOperationException) - { - logger.LogWarning(ex, "Failed to clean up import staging table {TempTableName}.", TempTableName); - } - } - private static async Task RunStagingValidationAsync( SqlConnection connection, SqlTransaction transaction, @@ -1047,7 +1034,25 @@ private async Task LogImportAttemptAsync( DateTimeOffset startedAt, CancellationToken cancellationToken) { - var importLog = new ImportLog + var importLog = CreateImportLog(dataset, filename, attemptedRows, rowsImported, status, errorPayload, user, startedAt); + + dbContext.ImportLogs.Add(importLog); + await dbContext.SaveChangesAsync(cancellationToken); + + return importLog.Id; + } + + private ImportLog CreateImportLog( + string dataset, + string? filename, + int attemptedRows, + int? rowsImported, + string status, + string? errorPayload, + ClaimsPrincipal? user, + DateTimeOffset startedAt) + { + return new ImportLog { Dataset = Truncate(dataset, 100), Filename = Truncate(Path.GetFileName(filename ?? string.Empty), 260), @@ -1061,11 +1066,6 @@ private async Task LogImportAttemptAsync( Status = status, ErrorPayload = errorPayload, }; - - dbContext.ImportLogs.Add(importLog); - await dbContext.SaveChangesAsync(cancellationToken); - - return importLog.Id; } private static string Truncate(string? value, int maxLength) diff --git a/server/appsettings.Development.json b/server/appsettings.Development.json index f188983..b641068 100644 --- a/server/appsettings.Development.json +++ b/server/appsettings.Development.json @@ -10,7 +10,8 @@ } }, "ConnectionStrings": { - "DefaultConnection": "Server=localhost,14333;Database=AppDb;User ID=sa;Password=LocalDev123!;Encrypt=False;TrustServerCertificate=True;" + "DefaultConnection": "Server=localhost,14333;Database=AppDb;User ID=sa;Password=LocalDev123!;Encrypt=False;TrustServerCertificate=True;", + "DataConnection": "Server=localhost,14333;Database=DataDb;User ID=sa;Password=LocalDev123!;Encrypt=False;TrustServerCertificate=True;" }, "Smtp": { "Host": "", diff --git a/tests/server.tests/Data/DataDbConnectionTests.cs b/tests/server.tests/Data/DataDbConnectionTests.cs new file mode 100644 index 0000000..a32405b --- /dev/null +++ b/tests/server.tests/Data/DataDbConnectionTests.cs @@ -0,0 +1,43 @@ +using FluentAssertions; +using Microsoft.Extensions.Configuration; +using Server.Core.Data; + +namespace Server.Tests.Data; + +public class DataDbConnectionTests +{ + [Fact] + public void Resolve_skips_blank_environment_variable_and_uses_named_connection() + { + var configuration = BuildConfiguration(new Dictionary + { + [DataDbConnection.EnvironmentVariableName] = " ", + [$"ConnectionStrings:{DataDbConnection.ConnectionStringName}"] = "named-connection", + }); + + var connectionString = DataDbConnection.Resolve(configuration, "fallback-connection"); + + connectionString.Should().Be("named-connection"); + } + + [Fact] + public void Resolve_skips_blank_configured_connections_and_uses_fallback() + { + var configuration = BuildConfiguration(new Dictionary + { + [DataDbConnection.EnvironmentVariableName] = " ", + [$"ConnectionStrings:{DataDbConnection.ConnectionStringName}"] = "\t", + }); + + var connectionString = DataDbConnection.Resolve(configuration, "fallback-connection"); + + connectionString.Should().Be("fallback-connection"); + } + + private static IConfiguration BuildConfiguration(Dictionary values) + { + return new ConfigurationBuilder() + .AddInMemoryCollection(values) + .Build(); + } +} diff --git a/tests/server.tests/Import/FlatFileImportServiceTests.cs b/tests/server.tests/Import/FlatFileImportServiceTests.cs index 67ff330..496d87e 100644 --- a/tests/server.tests/Import/FlatFileImportServiceTests.cs +++ b/tests/server.tests/Import/FlatFileImportServiceTests.cs @@ -1,6 +1,7 @@ using ClosedXML.Excel; using FluentAssertions; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Logging.Abstractions; using Server.Import; using Server.Models.Imports; @@ -452,6 +453,7 @@ private static FlatFileImportService CreateService(Server.Core.Data.AppDbContext return new FlatFileImportService( db, new FlatFileImportRegistry(), + new ConfigurationBuilder().Build(), NullLogger.Instance); }