diff --git a/.devcontainer/post-create.sh b/.devcontainer/post-create.sh index b1840f77c..242943eec 100644 --- a/.devcontainer/post-create.sh +++ b/.devcontainer/post-create.sh @@ -144,11 +144,11 @@ fi echo "Installing PowerShell modules..." pwsh -Command 'Install-Module HtmlToMarkdown -AcceptLicense -Force' pwsh -Command 'Install-Module Pester -Force -SkipPublisherCheck -MinimumVersion "5.0.0" -Scope CurrentUser' -# Az.Accounts + Az.Resources are the only Az sub-modules needed for Deploy-Infrastructure.ps1 -# (New-AzDeployment, Test-AzDeployment, Get-AzContext). Installing just these avoids pulling -# the full Az bundle (~80 modules). +# Install only the Az sub-modules required by the deployment scripts. +# Az.PostgreSql is needed for Get-AzPostgreSqlFlexibleServerFirewallRule in Deploy-Infrastructure.ps1. pwsh -Command 'Install-Module Az.Accounts -Force -Scope CurrentUser' pwsh -Command 'Install-Module Az.Resources -Force -Scope CurrentUser' +pwsh -Command 'Install-Module Az.PostgreSql -Force -Scope CurrentUser' # ==================== PowerShell Profile ==================== echo "Setting up PowerShell profile..." diff --git a/.github/prompts/address-th-review-comments.prompt.md b/.github/prompts/address-th-review-comments.prompt.md index d1ddf585d..20508ac6d 100644 --- a/.github/prompts/address-th-review-comments.prompt.md +++ b/.github/prompts/address-th-review-comments.prompt.md @@ -1,10 +1,10 @@ --- name: address-th-review-comments -description: "Reviews all open review comment threads on the current branch's pull request, analyses each one, applies code fixes where needed, replies to each thread explaining what was done or why it was ignored, resolves each thread after replying, then commits and pushes directly." +description: "Reviews all open review comment threads, CodeQL code scanning alerts, and GitHub Advanced Security alerts on the current branch's pull request, analyses each one, applies code fixes where needed, replies to each thread explaining what was done or why it was ignored, resolves each thread after replying, then commits and pushes directly." model: Claude Sonnet 4.6 --- -# Address PR Review Comments +# Address PR Review Comments, CodeQL Alerts, and Advanced Security Alerts **🚨 CRITICAL**: Read this entire prompt from start to finish before executing any step. @@ -103,7 +103,9 @@ Store the result as `[REPO]` (format: `owner/repo`). --- -## Step 5 β€” Fetch all open (unresolved) review threads +## Step 5 β€” Fetch all open issues (review threads + security alerts) + +### 5a β€” Fetch unresolved review threads Run the following GraphQL query to retrieve all unresolved review threads and their comments. Replace `[OWNER]`, `[REPONAME]`, and `[PR_NUMBER]` with the actual values (split `[REPO]` on `/`): @@ -171,21 +173,55 @@ gh api graphql -f query=' ' | Out-File -FilePath ".tmp/pr-review-threads.json" -Encoding utf8 ``` -Parse the file. Filter to threads where `isResolved` is `false`. If there are no unresolved threads, inform the user: +Parse the file. Filter to threads where `isResolved` is `false`. Count them and store as `[THREAD_COUNT]`. + +### 5b β€” Fetch open code scanning (CodeQL) alerts + +Fetch all open code scanning alerts for the branch: + +```pwsh +gh api "repos/[REPO]/code-scanning/alerts?ref=refs/heads/[BRANCHNAME]&state=open&per_page=100" | Out-File -FilePath ".tmp/pr-codeql-alerts.json" -Encoding utf8 +``` + +Parse the file. Each alert has: `number`, `rule.id`, `rule.description`, `rule.severity`, `most_recent_instance.location` (`path`, `start_line`, `end_line`), `most_recent_instance.message.text`, and `html_url`. + +If the command fails (e.g., code scanning is not enabled), record 0 alerts and continue. + +Count the open alerts and store as `[CODEQL_COUNT]`. + +### 5c β€” Fetch open secret scanning alerts + +Fetch all open secret scanning alerts: + +```pwsh +gh api "repos/[REPO]/secret-scanning/alerts?state=open&per_page=100" | Out-File -FilePath ".tmp/pr-secret-alerts.json" -Encoding utf8 +``` + +Parse the file. Each alert has: `number`, `secret_type_display_name`, `resolution`, `html_url`, and `locations_url`. + +If the command fails (e.g., secret scanning is not enabled), record 0 alerts and continue. + +Count the open alerts and store as `[SECRET_COUNT]`. + +### 5d β€” Summarise what was found -> All review threads on PR #[PR_NUMBER] are already resolved. Nothing to do. +If ALL three counts are zero, inform the user: -Then skip directly to Step 9. +> No open review threads, CodeQL alerts, or secret scanning alerts found on PR #[PR_NUMBER]. Nothing to do. -Otherwise, count the unresolved threads and report: +Then skip directly to Step 10. -> Found [N] unresolved review thread(s) to address. +Otherwise report: -**CHECKPOINT**: "βœ… Step 5 completed. Found [N] unresolved thread(s). Moving to Step 6." +> Found [THREAD_COUNT] unresolved review thread(s), [CODEQL_COUNT] open CodeQL alert(s), and [SECRET_COUNT] open secret scanning alert(s) to address. + +**CHECKPOINT**: "βœ… Step 5 completed. [THREAD_COUNT] thread(s), [CODEQL_COUNT] CodeQL alert(s), [SECRET_COUNT] secret alert(s). Moving to Step 6." --- -## Step 6 β€” Analyse and address each open thread +## Step 6 β€” Analyse and address each open review thread + +If `[THREAD_COUNT]` is 0, skip this step entirely. **🚨 CRITICAL**: Steps 6d (reply) and 6e (resolve) are **MANDATORY** for **every single thread** β€” including threads where you made a code fix. You are not done with a thread until you have BOTH posted a reply AND resolved it on GitHub. Never skip 6d or 6e. Never batch them for later. @@ -262,11 +298,132 @@ Confirm both actions completed, then state: Repeat steps 6a–6f for every unresolved thread before moving on. -**CHECKPOINT**: "βœ… Step 6 completed. All [N] threads replied to and resolved on GitHub. Moving to Step 7." +**CHECKPOINT**: "βœ… Step 6 completed. All [THREAD_COUNT] threads replied to and resolved on GitHub. Moving to Step 7." + +--- + +## Step 7 β€” Analyse and address each open CodeQL alert + +If `[CODEQL_COUNT]` is 0, skip this step entirely. + +Work through each open CodeQL alert one at a time. **Complete all sub-steps before moving to the next alert.** + +### 7a β€” Read and understand the alert + +For each alert, read: + +- **Rule**: `rule.id` and `rule.description` β€” what vulnerability or code quality issue was detected +- **Severity**: `rule.severity` (e.g., `error`, `warning`, `note`) +- **Location**: `most_recent_instance.location.path` and `start_line` β€” the exact file and line +- **Message**: `most_recent_instance.message.text` β€” the specific diagnostic message + +Read the relevant file around the flagged line to understand the current code in context. + +### 7b β€” Decide: fix or dismiss + +**NEEDS A FIX** β€” The alert points to a genuine security issue, vulnerability, or code defect that should be corrected (e.g., SQL injection risk, unvalidated input, exposed secret, use of a deprecated insecure API). + +**DISMISS** β€” The alert is a false positive, the code path is unreachable, the risk is mitigated elsewhere, or the flagged pattern is an intentional and safe design choice. + +**🚨 CRITICAL**: For `error`-severity alerts, default to fixing unless there is a clear, well-reasoned case for dismissal. + +### 7c β€” If NEEDS A FIX: apply the fix + +Make the minimal correct change to resolve the CodeQL finding. Follow the conventions in the relevant `AGENTS.md` files. Run `get_errors` after editing to ensure no new errors were introduced. + +### 7d β€” **MANDATORY**: Dismiss or note the alert on GitHub + +**If you fixed the code**: The alert will auto-close when the fix is pushed. No API call needed here β€” just make a note that this alert was fixed in code. + +**If dismissing (false positive / won't fix)**: Dismiss the alert via the API: + +```pwsh +gh api repos/[REPO]/code-scanning/alerts/[ALERT_NUMBER] -X PATCH -f state="dismissed" -f dismissed_reason="false positive" -f dismissed_comment="[One or two sentences explaining why this is a false positive or won't be fixed, referencing the specific code path or mitigation]" +``` + +Valid `dismissed_reason` values: `"false positive"`, `"won't fix"`, `"used in tests"`. + +Verify the command exits 0. If it fails, stop and report the error. + +### 7e β€” Checkpoint for this alert + +State: + +"βœ… CodeQL alert [N/TOTAL] (#[ALERT_NUMBER] β€” [rule.id]) β€” [FIXED in code / DISMISSED]. [Brief one-line summary of action taken]." --- -## Step 7 β€” Verify no new errors +Repeat steps 7a–7e for every open CodeQL alert before moving on. + +**CHECKPOINT**: "βœ… Step 7 completed. All [CODEQL_COUNT] CodeQL alerts addressed. Moving to Step 8." + +--- + +## Step 8 β€” Analyse and address each open secret scanning alert + +If `[SECRET_COUNT]` is 0, skip this step entirely. + +Work through each open secret scanning alert one at a time. + +### 8a β€” Read and understand the alert + +For each alert: + +- Note the `secret_type_display_name` (e.g., "GitHub Personal Access Token") +- Fetch the alert locations to find where in the codebase the secret appears: + + ```pwsh + gh api [LOCATIONS_URL] + ``` + +- Identify the file(s) and line(s) involved. + +### 8b β€” Decide: rotate or dismiss + +**ROTATE / REMEDIATE** β€” The secret is a real credential or token that should not be in source code. The correct fix is to: + +1. Remove the secret from the file (replace with an environment variable reference, a secrets manager reference, or a placeholder) +2. Immediately rotate/revoke the actual secret in the relevant system (GitHub, Azure Key Vault, etc.) β€” **inform the user** that rotation is needed since you cannot do this on their behalf + +**DISMISS** β€” The value is a test fixture, a clearly fake/placeholder value, or is already rotated and the alert is stale. + +**🚨 CRITICAL**: Never leave a real secret in the codebase. If in doubt, treat it as real and remediate. + +### 8c β€” If ROTATE / REMEDIATE: remove the secret from code + +Replace the hardcoded secret with an appropriate reference (e.g., `Environment.GetEnvironmentVariable("SECRET_NAME")`, a config binding, or a Key Vault reference). Follow existing patterns in the codebase. Run `get_errors` after editing. + +**Then inform the user**: + +> ⚠️ Secret `[SECRET_TYPE]` was found at `[FILE]:[LINE]`. The hardcoded value has been removed from the code. **You must manually rotate this secret** in [the relevant system] to ensure the exposed value is no longer valid. + +### 8d β€” **MANDATORY**: Resolve or dismiss the alert on GitHub + +**If you removed the secret from code**: The alert will auto-close when the fix is pushed. No API call needed. + +**If dismissing**: Dismiss via the API: + +```pwsh +gh api repos/[REPO]/secret-scanning/alerts/[ALERT_NUMBER] -X PATCH -f resolution="used_in_tests" -f resolution_comment="[Brief explanation]" +``` + +Valid `resolution` values: `"false_positive"`, `"wont_fix"`, `"revoked"`, `"used_in_tests"`. + +### 8e β€” Checkpoint for this alert + +State: + +"βœ… Secret alert [N/TOTAL] (#[ALERT_NUMBER] β€” [secret_type_display_name]) β€” [REMEDIATED in code / DISMISSED]. [Brief one-line summary]." + +--- + +Repeat steps 8a–8e for every open secret scanning alert before moving on. + +**CHECKPOINT**: "βœ… Step 8 completed. All [SECRET_COUNT] secret scanning alerts addressed. Moving to Step 9." + +--- + +## Step 9 β€” Verify no new errors Run: @@ -274,35 +431,55 @@ Run: Run -Clean ``` -If there are build or test failures caused by changes made in Step 6, fix them before proceeding. +If there are build or test failures caused by changes made in Steps 6, 7, or 8, fix them before proceeding. -**CHECKPOINT**: "βœ… Step 7 completed. No errors or failures. Moving to Step 8." +**CHECKPOINT**: "βœ… Step 9 completed. No errors or failures. Moving to Step 10." --- -## Step 8 β€” Summarise changes for the user +## Step 10 β€” Summarise changes for the user + +Print a concise summary with three sections: -Print a concise table of every thread and what was done: +**Review threads:** | # | File | Line | Action | Summary | -|---|------|------|--------|---------| +|---|------|------|--------|------| | 1 | ... | ... | Fixed / No fix | ... | -**CHECKPOINT**: "βœ… Step 8 completed. Summary provided. Moving to Step 9." +**CodeQL alerts:** + +| # | Alert # | Rule | Severity | Action | Summary | +|---|---------|------|----------|--------|------| +| 1 | ... | ... | ... | Fixed / Dismissed | ... | + +**Secret scanning alerts:** + +| # | Alert # | Type | Action | Summary | +|---|---------|------|--------|------| +| 1 | ... | ... | Remediated / Dismissed | ... | + +If a category had 0 items, omit its table and note "None found." + +**CHECKPOINT**: "βœ… Step 10 completed. Summary provided. Moving to Step 11." --- -## Step 9 β€” Commit and push directly +## Step 11 β€” Commit and push directly -If **no code changes** were made (all threads received "no fix needed" replies), skip this step β€” no commit is necessary. +If **no code changes** were made (all issues received "no fix" / "dismiss" responses), skip this step β€” no commit is necessary. -Otherwise, stage and commit only the files changed in Step 6: +Otherwise, stage all changed files: ```pwsh git add -A ``` -Write a short, direct commit message summarising the fixes (no ticket numbers, no PR references). Use imperative mood. Example: `"Address PR review comments: [brief summary]"`. +Write a short, direct commit message summarising the fixes (no ticket numbers, no PR references). Use imperative mood. Include all relevant issue types. Examples: + +- `"Address PR review comments: [brief summary]"` +- `"Fix CodeQL alerts: [brief summary]"` +- `"Address review comments and fix CodeQL alerts: [brief summary]"` ```pwsh git commit -m "[COMMIT_MESSAGE]" @@ -317,4 +494,4 @@ git push origin [BRANCHNAME] If the push is rejected for any reason, stop and ask the user. -**CHECKPOINT**: "βœ… Step 9 completed. Changes committed and pushed. Workflow complete." +**CHECKPOINT**: "βœ… Step 11 completed. Changes committed and pushed. Workflow complete." diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index 0660f498d..bbba5ee33 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -595,6 +595,11 @@ jobs: subscription-id: ${{ vars.AZURE_SUBSCRIPTION_ID }} enable-AzPSSession: true + - name: Install Az.PostgreSql module + if: steps.changes.outputs.infra-changed == 'true' + shell: pwsh + run: Install-Module Az.PostgreSql -Force -Scope CurrentUser -ErrorAction Stop + - name: Deploy infrastructure (Phase 1) if: steps.changes.outputs.infra-changed == 'true' shell: pwsh diff --git a/Directory.Packages.props b/Directory.Packages.props index 16e6ecf8a..b864a8923 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -2,7 +2,6 @@ true - @@ -11,6 +10,7 @@ + @@ -43,4 +43,4 @@ - + \ No newline at end of file diff --git a/docs/wildcard-certificates.md b/docs/wildcard-certificates.md index 5fc667065..9a1de9684 100644 --- a/docs/wildcard-certificates.md +++ b/docs/wildcard-certificates.md @@ -37,11 +37,11 @@ These are permanent β€” they never need updating. ### One-time GoDaddy NS delegation -After deploying the shared infrastructure (which creates the `acme.hub.ms` zone), add NS records for `acme` in the `hub.ms` zone at GoDaddy pointing to the Azure DNS nameservers: +After deploying the infrastructure (which creates the `acme.hub.ms` zone), add NS records for `acme` in the `hub.ms` zone at GoDaddy pointing to the Azure DNS nameservers: ```bash -# Get the nameservers after deploying shared infra -az network dns zone show --resource-group rg-techhub-shared --name acme.hub.ms --query nameServers -o tsv +# Get the nameservers after deploying infra +az network dns zone show --resource-group rg-techhub-prod --name acme.hub.ms --query nameServers -o tsv ``` Add each returned nameserver as an NS record for `acme` in GoDaddy's `hub.ms` zone. @@ -57,8 +57,8 @@ pip install certbot certbot-dns-azure The ACME DNS zone is deployed as part of shared infrastructure: - **Bicep module**: `infra/modules/acmeDnsZone.bicep` -- **Deployed by**: `infra/shared.bicep` β†’ `acmeDnsZone` module -- **Resource**: `Microsoft.Network/dnsZones` β€” `acme.hub.ms` in `rg-techhub-shared` +- **Deployed by**: `infra/infrastructure.bicep` β†’ `acmeDnsZone` module +- **Resource**: `Microsoft.Network/dnsZones` β€” `acme.hub.ms` in `rg-techhub-prod` Deploy with: diff --git a/infra/infrastructure.bicep b/infra/infrastructure.bicep index 6e74e00d4..c9c4876ef 100644 --- a/infra/infrastructure.bicep +++ b/infra/infrastructure.bicep @@ -80,6 +80,12 @@ param emailServiceName string = 'eml-techhub-prod' @description('Communication Service name') param communicationServiceName string = 'acs-techhub-prod' +@description('Custom email sending domain linked to ACS (e.g. mail.hub.ms). Must be verified in ACS after deploy.') +param customEmailDomain string = 'mail.hub.ms' + +@description('Set to true only after the custom domain DNS records have been added and verified in ACS. Redeploy after verification.') +param linkEmailDomain bool = false + @description('Common tags applied to all resources managed by this template') param commonTags object = { owner: 'techhub-maintainer' @@ -305,6 +311,8 @@ module communication './modules/communication.bicep' = { name: 'communication-${deploymentSuffix}' params: { emailServiceName: emailServiceName + customEmailDomain: customEmailDomain + linkEmailDomain: linkEmailDomain communicationServiceName: communicationServiceName tags: prodTags } diff --git a/infra/modules/communication.bicep b/infra/modules/communication.bicep index b414722d0..00dd0a813 100644 --- a/infra/modules/communication.bicep +++ b/infra/modules/communication.bicep @@ -1,6 +1,8 @@ param emailServiceName string -param managedDomainName string = 'AzureManagedDomain' +param customEmailDomain string param communicationServiceName string +@description('Set to true only after the custom domain DNS records have been added and verified in ACS.') +param linkEmailDomain bool = false param tags object = {} resource emailService 'Microsoft.Communication/emailServices@2026-03-18' = { @@ -14,26 +16,33 @@ resource emailService 'Microsoft.Communication/emailServices@2026-03-18' = { resource domain 'Microsoft.Communication/emailServices/domains@2026-03-18' = { parent: emailService - name: managedDomainName + name: customEmailDomain location: 'global' properties: { - domainManagement: 'AzureManaged' + domainManagement: 'CustomerManaged' userEngagementTracking: 'Disabled' } } +resource newsletterSenderUsername 'Microsoft.Communication/emailServices/domains/senderUsernames@2026-03-18' = { + parent: domain + name: 'newsletter' + properties: { + username: 'newsletter' + displayName: 'TechHub Newsletter' + } +} + resource communicationService 'Microsoft.Communication/communicationServices@2026-03-18' = { name: communicationServiceName location: 'global' tags: tags properties: { dataLocation: 'Europe' - linkedDomains: [ - domain.id - ] + linkedDomains: linkEmailDomain ? [domain.id] : [] } } output communicationServiceEndpoint string = 'https://${communicationService.name}.communication.azure.com/' output communicationServiceId string = communicationService.id -output senderAddress string = 'DoNotReply@${domain.properties.mailFromSenderDomain}' +output senderAddress string = 'newsletter@${domain.properties.mailFromSenderDomain}' diff --git a/infra/parameters/prod-infrastructure.bicepparam b/infra/parameters/prod-infrastructure.bicepparam index 1125b91f6..797bdbfdc 100644 --- a/infra/parameters/prod-infrastructure.bicepparam +++ b/infra/parameters/prod-infrastructure.bicepparam @@ -25,3 +25,4 @@ param adminIpAddresses = readEnvironmentVariable('ADMIN_IP_ADDRESSES') param alertEmailAddress = 'reinier.vanmaanen@xebia.com' param monthlyBudgetAmount = 250 param budgetStartDate = '2026-04-01' +param linkEmailDomain = true diff --git a/scripts/Deploy-Infrastructure.ps1 b/scripts/Deploy-Infrastructure.ps1 index 110f55f48..c380c7d4a 100644 --- a/scripts/Deploy-Infrastructure.ps1 +++ b/scripts/Deploy-Infrastructure.ps1 @@ -37,7 +37,10 @@ param( [string]$Mode = 'whatif', [Parameter(Mandatory = $false)] - [string]$Location = 'swedencentral' + [string]$Location = 'swedencentral', + + [Parameter(Mandatory = $false)] + [switch]$LinkEmailDomain ) $ErrorActionPreference = "Stop" @@ -222,18 +225,33 @@ if ($Mode -eq 'deploy') { $savedVerbose = $VerbosePreference $VerbosePreference = 'Continue' try { - New-AzDeployment ` - -Name $deploymentName ` - -Location $Location ` - -TemplateFile $infraTemplateFile ` - -TemplateParameterFile $infraParamsFile ` - -SkipTemplateParameterPrompt ` - -OutVariable infraResult | Out-Null + $effectiveParamsFile = $infraParamsFile + $tempParamsFile = $null + if ($LinkEmailDomain) { + Write-Warn "LinkEmailDomain is set β€” domain will be linked to ACS. Only use this after DNS verification." + # Write temp file to the same directory so the 'using' relative path remains valid + $tempParamsFile = Join-Path (Split-Path $infraParamsFile) "prod-infrastructure-linkdomain.bicepparam" + (Get-Content $infraParamsFile -Raw) ` + -replace 'param linkEmailDomain = (?:false|true)', 'param linkEmailDomain = true' | + Set-Content $tempParamsFile + $effectiveParamsFile = $tempParamsFile + } + $deployParams = @{ + Name = $deploymentName + Location = $Location + TemplateFile = $infraTemplateFile + TemplateParameterFile = $effectiveParamsFile + SkipTemplateParameterPrompt = $true + } + New-AzDeployment @deployParams -OutVariable infraResult | Out-Null } catch { Write-Fail "Infrastructure deployment failed: $_" exit 1 } finally { $VerbosePreference = $savedVerbose + if ($tempParamsFile -and (Test-Path $tempParamsFile)) { + Remove-Item $tempParamsFile -Force + } } Write-Ok "Base infrastructure deployed successfully" diff --git a/scripts/Renew-WildcardCertificates.ps1 b/scripts/Renew-WildcardCertificates.ps1 index c7d2f1efe..814f5d59c 100644 --- a/scripts/Renew-WildcardCertificates.ps1 +++ b/scripts/Renew-WildcardCertificates.ps1 @@ -12,8 +12,8 @@ Prerequisites: - certbot and certbot-dns-azure installed (pip install certbot certbot-dns-azure) - - Azure CLI authenticated with access to rg-techhub-shared - - ACME DNS zone deployed (acme.hub.ms in rg-techhub-shared) + - Azure CLI authenticated with access to rg-techhub-prod + - ACME DNS zone deployed (acme.hub.ms in rg-techhub-prod) - GoDaddy CNAME records configured (see docs/wildcard-certificates.md) .PARAMETER KeyVaultName @@ -51,7 +51,7 @@ param( [string]$KeyVaultName = 'kv-techhub-prod', [Parameter(Mandatory = $false)] - [string]$ResourceGroup = 'rg-techhub-shared', + [string]$ResourceGroup = 'rg-techhub-prod', [Parameter(Mandatory = $false)] [string]$AcmeDnsZone = 'acme.hub.ms', @@ -147,7 +147,7 @@ Write-Ok "certbot-dns-azure plugin found" az network dns zone show --resource-group $ResourceGroup --name $AcmeDnsZone -o json 2>&1 | Out-Null if ($LASTEXITCODE -ne 0) { Write-Fail "DNS zone '$AcmeDnsZone' not found in resource group '$ResourceGroup'." - Write-Detail "Deploy shared infrastructure first: ./scripts/Deploy-Infrastructure.ps1 -Environment shared -Mode deploy" + Write-Detail "Deploy infrastructure first: ./scripts/Deploy-Infrastructure.ps1 -Mode deploy" exit 1 } Write-Ok "ACME DNS zone '$AcmeDnsZone' exists" diff --git a/src/AGENTS.md b/src/AGENTS.md index 014b0c1b1..b5b01d5ff 100644 --- a/src/AGENTS.md +++ b/src/AGENTS.md @@ -45,6 +45,32 @@ - **End users get friendly error messages** β€” But logs and Application Insights must contain full exception details, parameters, and context - **Log then rethrow or let it crash** β€” Prefer logging with context and rethrowing over catching and returning a default value +### Accepted Broad-Catch Pattern + +Catching all non-cancellation exceptions (`when (ex is not OperationCanceledException)`) is **accepted and preferred** in two scenarios: + +1. **Background service loops** β€” To prevent the entire service (and thus the application) from crashing on one unexpected error. The service logs, then continues processing on the next tick. + + ```csharp + catch (Exception ex) when (ex is not OperationCanceledException) + { + _logger.LogError(ex, "Unhandled exception in … β€” service will continue"); + } + ``` + +2. **Per-item processing loops** β€” To skip a failing item and continue with the next (e.g., content processing, newsletter subscriber sending). Must be suppressed with `#pragma warning disable CA1031` and a comment explaining the intent. + + ```csharp + #pragma warning disable CA1031 // Best-effort: continue with other items if one fails + catch (Exception ex) when (ex is not OperationCanceledException) + { + _logger.LogWarning(ex, "Failed to process {Item} β€” skipping", item); + } + #pragma warning restore CA1031 + ``` + +Do **not** add `OutOfMemoryException` or `StackOverflowException` exclusions β€” the CLR already terminates the process for those regardless of catch blocks. + ## Project Structure ```text diff --git a/src/TechHub.Api/AGENTS.md b/src/TechHub.Api/AGENTS.md index df7e5fdfe..faad254bf 100644 --- a/src/TechHub.Api/AGENTS.md +++ b/src/TechHub.Api/AGENTS.md @@ -6,7 +6,18 @@ REST API backend using ASP.NET Core Minimal APIs. Exposes endpoints for sections, content, filtering, RSS feeds, and structured data. -**Testing**: See [tests/TechHub.Api.Tests/AGENTS.md](../../tests/TechHub.Api.Tests/AGENTS.md) for integration testing patterns. +## Background Services + +Background services inherit from `BackgroundService` and are registered in `Program.cs` via `AddHostedService()`. The outer execution loop MUST catch all non-cancellation exceptions to prevent the entire application from crashing: + +```csharp +catch (Exception ex) when (ex is not OperationCanceledException) +{ + _logger.LogError(ex, "Unhandled exception in MyBackgroundService β€” service will continue"); +} +``` + +See [src/AGENTS.md](../AGENTS.md#accepted-broad-catch-pattern) for the accepted broad-catch pattern and rationale. ## Project Structure diff --git a/src/TechHub.Api/Program.cs b/src/TechHub.Api/Program.cs index 81d5a2ec0..3426669fb 100644 --- a/src/TechHub.Api/Program.cs +++ b/src/TechHub.Api/Program.cs @@ -261,6 +261,7 @@ builder.Configuration.GetSection(NewsletterOptions.SectionName)); builder.Services.AddScoped(); builder.Services.AddScoped(); +builder.Services.AddSingleton(); builder.Services.AddScoped(); builder.Services.AddAcsEmailClient(); builder.Services.AddScoped(); diff --git a/src/TechHub.Api/Services/ContentFixerBackgroundService.cs b/src/TechHub.Api/Services/ContentFixerBackgroundService.cs index b026acc8f..091bb006a 100644 --- a/src/TechHub.Api/Services/ContentFixerBackgroundService.cs +++ b/src/TechHub.Api/Services/ContentFixerBackgroundService.cs @@ -124,7 +124,7 @@ await jobRepo.AbortJobAsync(jobId, transcriptsSucceeded: 0, transcriptsFailed: 0, logOutput: "Content cleanup aborted by admin.", ct: CancellationToken.None); } - catch (Exception ex) when (ex is not OperationCanceledException and not OutOfMemoryException and not StackOverflowException) + catch (Exception ex) when (ex is not OperationCanceledException) { _logger.LogError(ex, "Content fixer run failed (job {JobId})", jobId); await jobRepo.FailAsync(jobId, @@ -137,7 +137,7 @@ await jobRepo.FailAsync(jobId, { // Shutting down β€” expected } - catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) + catch (Exception ex) when (ex is not OperationCanceledException) { _logger.LogError(ex, "Unexpected exception in ContentFixerBackgroundService"); } diff --git a/src/TechHub.Api/Services/ContentProcessingBackgroundService.cs b/src/TechHub.Api/Services/ContentProcessingBackgroundService.cs index a89d6c308..235ae2253 100644 --- a/src/TechHub.Api/Services/ContentProcessingBackgroundService.cs +++ b/src/TechHub.Api/Services/ContentProcessingBackgroundService.cs @@ -172,7 +172,7 @@ private async Task RunOnceAsync(string triggerType, CancellationToken ct) { // Shutting down β€” expected } - catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) + catch (Exception ex) when (ex is not OperationCanceledException) { // Errors are already recorded by ContentProcessingService; log defensively here _logger.LogError(ex, "Unexpected exception in ContentProcessingBackgroundService"); diff --git a/src/TechHub.Api/Services/NewsletterBackgroundService.cs b/src/TechHub.Api/Services/NewsletterBackgroundService.cs index cb3d049d3..0b122a130 100644 --- a/src/TechHub.Api/Services/NewsletterBackgroundService.cs +++ b/src/TechHub.Api/Services/NewsletterBackgroundService.cs @@ -76,7 +76,15 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) if (completed == manualTask) { _manualTrigger = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - await RunManualAsync(stoppingToken); + try + { + await RunManualAsync(stoppingToken); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + _logger.LogError(ex, "Unhandled exception in newsletter manual run β€” service will continue"); + } + continue; } @@ -87,8 +95,15 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) continue; } - await SendLatestRoundupsAsync(stoppingToken); - await SendScheduledDailyEmailsAsync(stoppingToken); + try + { + await SendLatestRoundupsAsync(stoppingToken); + await SendScheduledDailyEmailsAsync(stoppingToken); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + _logger.LogError(ex, "Unhandled exception in newsletter periodic run β€” service will continue"); + } } } diff --git a/src/TechHub.Api/Services/RoundupGeneratorBackgroundService.cs b/src/TechHub.Api/Services/RoundupGeneratorBackgroundService.cs index b6d857b54..5734ae44d 100644 --- a/src/TechHub.Api/Services/RoundupGeneratorBackgroundService.cs +++ b/src/TechHub.Api/Services/RoundupGeneratorBackgroundService.cs @@ -293,7 +293,7 @@ await jobRepo.AbortJobAsync(jobId, feedsProcessed: 0, itemsAdded: 0, itemsSkippe { // Shutting down β€” expected } - catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) + catch (Exception ex) when (ex is not OperationCanceledException) { _logger.LogError(ex, "Unexpected exception in RoundupGeneratorBackgroundService (job {JobId})", jobId); await jobRepo.AppendLogAsync(jobId, $"Roundup generation failed: {ex.Message}\n{ex.StackTrace}", CancellationToken.None); diff --git a/src/TechHub.Api/appsettings.json b/src/TechHub.Api/appsettings.json index 59f1691d2..4a5b989f1 100644 --- a/src/TechHub.Api/appsettings.json +++ b/src/TechHub.Api/appsettings.json @@ -429,7 +429,7 @@ "AdminReportEmailAddress": "whistler112@gmail.com", "DailyDigestHourLocal": 9, "DailyDigestTimeZoneId": "Europe/Brussels", - "SendDelayMs": 200 + "SendDelayMs": 2500 }, "AiCategorization": { "Endpoint": "", diff --git a/src/TechHub.Core/Interfaces/INewsletterSubscriberRepository.cs b/src/TechHub.Core/Interfaces/INewsletterSubscriberRepository.cs index 1916c6cd8..9fac6762a 100644 --- a/src/TechHub.Core/Interfaces/INewsletterSubscriberRepository.cs +++ b/src/TechHub.Core/Interfaces/INewsletterSubscriberRepository.cs @@ -52,6 +52,7 @@ Task LogSendAsync( string sendKind, string targetKey, int recipientCount, + int failedCount, string status, string? errorMessage, CancellationToken ct = default); diff --git a/src/TechHub.Core/Models/Admin/NewsletterDailyReportStats.cs b/src/TechHub.Core/Models/Admin/NewsletterDailyReportStats.cs index 443ef0ce7..aef4dbdb3 100644 --- a/src/TechHub.Core/Models/Admin/NewsletterDailyReportStats.cs +++ b/src/TechHub.Core/Models/Admin/NewsletterDailyReportStats.cs @@ -8,4 +8,5 @@ public sealed class NewsletterDailyReportStats public int NewSubscribersLast24Hours { get; init; } public int ActiveSubscribers { get; init; } public int UnconfirmedSubscribers { get; init; } + public int FailedNewsletterSendsLast24Hours { get; init; } } diff --git a/src/TechHub.Core/Models/Admin/NewsletterSendLogEntry.cs b/src/TechHub.Core/Models/Admin/NewsletterSendLogEntry.cs index 66b29ce78..cb931cf5b 100644 --- a/src/TechHub.Core/Models/Admin/NewsletterSendLogEntry.cs +++ b/src/TechHub.Core/Models/Admin/NewsletterSendLogEntry.cs @@ -7,6 +7,7 @@ public sealed class NewsletterSendLogEntry public string TargetKey { get; init; } = string.Empty; public DateTimeOffset SentAt { get; init; } public int RecipientCount { get; init; } + public int FailedCount { get; init; } public string Status { get; init; } = string.Empty; public string? ErrorMessage { get; init; } } diff --git a/src/TechHub.Infrastructure/AGENTS.md b/src/TechHub.Infrastructure/AGENTS.md index 42438a8f4..cfdf6e01a 100644 --- a/src/TechHub.Infrastructure/AGENTS.md +++ b/src/TechHub.Infrastructure/AGENTS.md @@ -55,6 +55,22 @@ See [Data/Migrations/postgres/](Data/Migrations/postgres/) for complete schema. | Scoped | `IDbConnection` (per-request) | | Transient | All other services | +## Per-Item Processing Loops + +Processing pipelines (content ingestion, newsletter sending, roundup generation) iterate over collections and MUST NOT abort the entire run when one item fails. Use the broad-catch pattern with `#pragma warning disable CA1031`: + +```csharp +#pragma warning disable CA1031 // Best-effort: continue with other items if one fails +catch (Exception ex) when (ex is not OperationCanceledException) +{ + errorCount++; + _logger.LogWarning(ex, "Failed to process {Item} β€” skipping", item); +} +#pragma warning restore CA1031 +``` + +See [src/AGENTS.md](../AGENTS.md#accepted-broad-catch-pattern) for the full rationale. + ## Tag Cloud `TagCloudService` queries tags with section/collection title exclusion (repository filters these BEFORE counting). Quantile sizing: top 25% Large, middle 50% Medium, bottom 25% Small. Size group normalization: 1 group β†’ all Medium, 2 β†’ Medium + Small. diff --git a/src/TechHub.Infrastructure/Data/DapperExtensions.cs b/src/TechHub.Infrastructure/Data/DapperExtensions.cs index e20849e7e..bf8a7b055 100644 --- a/src/TechHub.Infrastructure/Data/DapperExtensions.cs +++ b/src/TechHub.Infrastructure/Data/DapperExtensions.cs @@ -151,7 +151,7 @@ private static void LogExplainPlan(IDbConnection connection, string sql, object? logger.LogWarning("{ExplainPlan}", sb.ToString().TrimEnd()); } - catch (Exception ex) when (ex is not OutOfMemoryException) + catch (Exception ex) when (ex is not OperationCanceledException) { logger.LogWarning(ex, "Failed to retrieve EXPLAIN plan for slow query"); } diff --git a/src/TechHub.Infrastructure/Data/Migrations/postgres/046_newsletter_send_log_failed_count.sql b/src/TechHub.Infrastructure/Data/Migrations/postgres/046_newsletter_send_log_failed_count.sql new file mode 100644 index 000000000..371c1a1ce --- /dev/null +++ b/src/TechHub.Infrastructure/Data/Migrations/postgres/046_newsletter_send_log_failed_count.sql @@ -0,0 +1,2 @@ +ALTER TABLE newsletter_send_log + ADD COLUMN IF NOT EXISTS failed_count INT NOT NULL DEFAULT 0; diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-account-action-content.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-account-action-content.html new file mode 100644 index 000000000..dc724e890 --- /dev/null +++ b/src/TechHub.Infrastructure/Data/Resources/newsletter-account-action-content.html @@ -0,0 +1,5 @@ +

{{message}}

+

+ {{actionLabel}} → +

+

Or copy this link into your browser:
{{actionUrl}}

diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-admin-content.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-admin-content.html new file mode 100644 index 000000000..24eb03d60 --- /dev/null +++ b/src/TechHub.Infrastructure/Data/Resources/newsletter-admin-content.html @@ -0,0 +1,7 @@ +

New content items (24h): {{newContentItems}}

+

Failed processed URLs (24h): {{failedProcessedUrls}}

+

Failed background jobs (24h): {{failedJobs}}

+

Failed newsletter sends (24h): {{failedNewsletterSends}}

+

New newsletter subscribers (24h): {{newSubscribers}}

+

Active subscribers: {{activeSubscribers}}

+

Unconfirmed subscribers: {{unconfirmedSubscribers}}

diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-admin-status-template.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-admin-status-template.html deleted file mode 100644 index fc6759a5c..000000000 --- a/src/TechHub.Infrastructure/Data/Resources/newsletter-admin-status-template.html +++ /dev/null @@ -1,52 +0,0 @@ - - - - - - - - - - - - - -
- - - - - - - -
TechHub Daily Status Report β€” {Date}
- - - - - - - - - - - - - - - - - - - - - - - - - -
New content items (24h){NewContentItems}
Failed processed URLs (24h){FailedUrls}
Failed background jobs (24h){FailedJobs}
New newsletter subscribers (24h){NewSubscribers}
Active subscribers{ActiveSubscribers}
Unconfirmed subscribers{UnconfirmedSubscribers}
-
-
- - diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-daily-content.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-daily-content.html new file mode 100644 index 000000000..fc427d5cd --- /dev/null +++ b/src/TechHub.Infrastructure/Data/Resources/newsletter-daily-content.html @@ -0,0 +1,13 @@ +

{{date}}

+{{#each sections}} +

{{title}}

+
    + {{#each items}} +
  • + {{title}} + ({{collectionName}}) +
    • + {{/each}} +
+{{/each}} +{{{footer}}} diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-daily-overview-template.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-daily-overview-template.html deleted file mode 100644 index 931974c31..000000000 --- a/src/TechHub.Infrastructure/Data/Resources/newsletter-daily-overview-template.html +++ /dev/null @@ -1,33 +0,0 @@ - - - - - - - - - - - - - -
- - - - - - - -
TechHub Daily Overview
-

{Date}

- {Content} -

- You received this because you subscribed to TechHub newsletters.
- Manage subscription  Β·  - Unsubscribe -

-
-
- - diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-subscriber-footer.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-subscriber-footer.html new file mode 100644 index 000000000..a3f4036df --- /dev/null +++ b/src/TechHub.Infrastructure/Data/Resources/newsletter-subscriber-footer.html @@ -0,0 +1,5 @@ +

+ You received this because you subscribed to TechHub newsletters.
+ Manage subscription  Β·  + Unsubscribe +

diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-template.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-template.html new file mode 100644 index 000000000..51c345601 --- /dev/null +++ b/src/TechHub.Infrastructure/Data/Resources/newsletter-template.html @@ -0,0 +1,63 @@ + + + + + + + + + + + + + + +
+ + + + + + + + +
+ + diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-weekly-content.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-weekly-content.html new file mode 100644 index 000000000..0914b4dbc --- /dev/null +++ b/src/TechHub.Infrastructure/Data/Resources/newsletter-weekly-content.html @@ -0,0 +1,26 @@ +{{#each roundups}} +{{#unless @first}}
{{/unless}} +
+

{{sectionTitle}}

+

{{title}}

+

{{introduction}}

+

In this roundup

+
+
    + {{#each links}} +
  • {{text}}
    • + {{/each}} +
+
+
+ + + + +
+ πŸ“‘ Read the full {{sectionTitle}} roundup → +
+
+
+{{/each}} +{{{footer}}} diff --git a/src/TechHub.Infrastructure/Data/Resources/newsletter-weekly-digest-template.html b/src/TechHub.Infrastructure/Data/Resources/newsletter-weekly-digest-template.html deleted file mode 100644 index bf4935043..000000000 --- a/src/TechHub.Infrastructure/Data/Resources/newsletter-weekly-digest-template.html +++ /dev/null @@ -1,32 +0,0 @@ - - - - - - - - - - - - - -
- - - - - - - -
TechHub Weekly Digest
- {Content} -

- You received this because you subscribed to TechHub newsletters.
- Manage subscription  Β·  - Unsubscribe -

-
-
- - diff --git a/src/TechHub.Infrastructure/Repositories/NewsletterSubscriberRepository.cs b/src/TechHub.Infrastructure/Repositories/NewsletterSubscriberRepository.cs index 9032434d5..bedbe1e7f 100644 --- a/src/TechHub.Infrastructure/Repositories/NewsletterSubscriberRepository.cs +++ b/src/TechHub.Infrastructure/Repositories/NewsletterSubscriberRepository.cs @@ -271,6 +271,7 @@ public async Task LogSendAsync( string sendKind, string targetKey, int recipientCount, + int failedCount, string status, string? errorMessage, CancellationToken ct = default) @@ -281,13 +282,14 @@ public async Task LogSendAsync( const string Sql = """ INSERT INTO newsletter_send_log - (send_kind, target_key, recipient_count, status, error_message) + (send_kind, target_key, recipient_count, failed_count, status, error_message) VALUES - (@SendKind, @TargetKey, @RecipientCount, @Status, @Error) + (@SendKind, @TargetKey, @RecipientCount, @FailedCount, @Status, @Error) ON CONFLICT (send_kind, target_key) DO UPDATE SET sent_at = NOW(), recipient_count = EXCLUDED.recipient_count, + failed_count = EXCLUDED.failed_count, status = EXCLUDED.status, error_message = EXCLUDED.error_message """; @@ -299,6 +301,7 @@ await _connection.ExecuteAsync(new CommandDefinition( SendKind = sendKind, TargetKey = targetKey, RecipientCount = recipientCount, + FailedCount = failedCount, Status = status, Error = errorMessage }, @@ -314,6 +317,7 @@ public async Task> GetSendLogAsync(int cou target_key AS TargetKey, sent_at AS SentAt, recipient_count AS RecipientCount, + failed_count AS FailedCount, status AS Status, error_message AS ErrorMessage FROM newsletter_send_log @@ -395,7 +399,8 @@ public async Task GetDailyReportStatsAsync(Cancellat (SELECT COUNT(*) FROM content_processing_jobs WHERE status = 'failed' AND started_at >= NOW() - INTERVAL '24 hours') AS FailedJobsLast24Hours, (SELECT COUNT(*) FROM newsletter_subscribers WHERE subscribed_at >= NOW() - INTERVAL '24 hours') AS NewSubscribersLast24Hours, (SELECT COUNT(*) FROM newsletter_subscribers WHERE is_confirmed = TRUE AND unsubscribed_at IS NULL) AS ActiveSubscribers, - (SELECT COUNT(*) FROM newsletter_subscribers WHERE is_confirmed = FALSE AND unsubscribed_at IS NULL) AS UnconfirmedSubscribers + (SELECT COUNT(*) FROM newsletter_subscribers WHERE is_confirmed = FALSE AND unsubscribed_at IS NULL) AS UnconfirmedSubscribers, + (SELECT COALESCE(SUM(failed_count), 0) FROM newsletter_send_log WHERE sent_at >= NOW() - INTERVAL '24 hours') AS FailedNewsletterSendsLast24Hours """; return await _connection.QuerySingleAsync(new CommandDefinition(Sql, cancellationToken: ct)); diff --git a/src/TechHub.Infrastructure/Services/ContentProcessing/ArticleContentService.cs b/src/TechHub.Infrastructure/Services/ContentProcessing/ArticleContentService.cs index bbe899df3..b9357a4b2 100644 --- a/src/TechHub.Infrastructure/Services/ContentProcessing/ArticleContentService.cs +++ b/src/TechHub.Infrastructure/Services/ContentProcessing/ArticleContentService.cs @@ -127,7 +127,7 @@ private static string ExtractMainContent(string html, string? url = null) return article.IsReadable ? article.Content : null; } - catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) + catch (Exception ex) when (ex is not OperationCanceledException) { return null; } diff --git a/src/TechHub.Infrastructure/Services/ContentProcessing/ContentProcessingService.cs b/src/TechHub.Infrastructure/Services/ContentProcessing/ContentProcessingService.cs index 8c24251ee..01273c707 100644 --- a/src/TechHub.Infrastructure/Services/ContentProcessing/ContentProcessingService.cs +++ b/src/TechHub.Infrastructure/Services/ContentProcessing/ContentProcessingService.cs @@ -444,7 +444,7 @@ await _jobRepo.CompleteAsync(jobId, feedsProcessed: 0, itemsAdded, itemsSkipped, await TryAbortJobAsync(jobId, 0, 0, 0, 0, 0, 0, log.ToString()); throw; } - catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) + catch (Exception ex) when (ex is not OperationCanceledException) { Log(string.Create(CultureInfo.InvariantCulture, $"FATAL ERROR: {ex.Message.Sanitize()}")); _logger.LogError(ex, "Ad-hoc processing job {JobId} failed with unhandled exception", jobId); diff --git a/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs b/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs index e4664e568..8dda8112d 100644 --- a/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs +++ b/src/TechHub.Infrastructure/Services/Newsletter/AcsEmailSender.cs @@ -45,7 +45,22 @@ public async Task SendAsync(string recipientEmail, string subject, string }, recipients: new EmailRecipients([new EmailAddress(recipientEmail)])); - var operation = await _emailClient.SendAsync(WaitUntil.Completed, emailMessage, ct); + EmailSendOperation operation; + try + { + operation = await _emailClient.SendAsync(WaitUntil.Completed, emailMessage, ct); + } + catch (RequestFailedException ex) when (ex.Status == 429) + { + _logger.LogWarning(ex, "Newsletter email send rate-limited by ACS (429)"); + return false; + } + catch (RequestFailedException ex) + { + _logger.LogError(ex, "Newsletter email send failed with ACS error {Status}", ex.Status); + return false; + } + if (operation.Value.Status != EmailSendStatus.Succeeded) { _logger.LogWarning("Newsletter email delivery completed with non-success status {Status}", operation.Value.Status); diff --git a/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs b/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs index 3022ffe6d..f00bba2d4 100644 --- a/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs +++ b/src/TechHub.Infrastructure/Services/Newsletter/NewsletterService.cs @@ -1,8 +1,6 @@ using System.Data; using System.Diagnostics.CodeAnalysis; using System.Globalization; -using System.Net; -using System.Reflection; using System.Security.Cryptography; using System.Text; using Dapper; @@ -27,9 +25,7 @@ public sealed class NewsletterService : INewsletterService private readonly AppSettings _appSettings; private readonly IEmailSender _emailSender; private readonly ILogger _logger; - private readonly string _weeklyDigestTemplate; - private readonly string _dailyOverviewTemplate; - private readonly string _adminStatusTemplate; + private readonly NewsletterTemplateProvider _templates; public NewsletterService( IDbConnection connection, @@ -38,7 +34,8 @@ public NewsletterService( IOptions options, IOptions appSettings, IEmailSender emailSender, - ILogger logger) + ILogger logger, + NewsletterTemplateProvider templates) { ArgumentNullException.ThrowIfNull(connection); ArgumentNullException.ThrowIfNull(subscriberRepository); @@ -47,6 +44,7 @@ public NewsletterService( ArgumentNullException.ThrowIfNull(appSettings); ArgumentNullException.ThrowIfNull(emailSender); ArgumentNullException.ThrowIfNull(logger); + ArgumentNullException.ThrowIfNull(templates); _connection = connection; _subscriberRepository = subscriberRepository; @@ -55,9 +53,7 @@ public NewsletterService( _appSettings = appSettings.Value; _emailSender = emailSender; _logger = logger; - _weeklyDigestTemplate = LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-weekly-digest-template.html"); - _dailyOverviewTemplate = LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-daily-overview-template.html"); - _adminStatusTemplate = LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-admin-status-template.html"); + _templates = templates; } public async Task SendCombinedWeeklyAsync(IReadOnlyList roundupSlugs, string? sendTargetKey = null, CancellationToken ct = default) @@ -95,7 +91,7 @@ public async Task SendCombinedWeeklyAsync(IReadOnlyList roundupSlu if (!IsUnsubscribeSecretConfigured("combined-weekly")) { - await _subscriberRepository.LogSendAsync("weekly-roundup", targetKey, 0, "failed", "Unsubscribe secret is not configured", ct); + await _subscriberRepository.LogSendAsync("weekly-roundup", targetKey, 0, 0, "failed", "Unsubscribe secret is not configured", ct); return false; } @@ -115,24 +111,24 @@ public async Task SendCombinedWeeklyAsync(IReadOnlyList roundupSlu var attempted = 0; var successful = 0; - try + foreach (var subscriber in allSubscribers.Values) { - foreach (var subscriber in allSubscribers.Values) + // Find the roundups this subscriber cares about (intersection of their sections and new roundups) + var relevantRoundups = subscriber.WeeklySections + .Where(s => roundupsBySection.ContainsKey(s)) + .Select(s => roundupsBySection[s]) + .ToList(); + relevantRoundups = OrderRoundupsByWebsiteOrder(relevantRoundups); + + if (relevantRoundups.Count == 0) { - // Find the roundups this subscriber cares about (intersection of their sections and new roundups) - var relevantRoundups = subscriber.WeeklySections - .Where(s => roundupsBySection.ContainsKey(s)) - .Select(s => roundupsBySection[s]) - .ToList(); - relevantRoundups = OrderRoundupsByWebsiteOrder(relevantRoundups); - - if (relevantRoundups.Count == 0) - { - continue; - } + continue; + } - attempted++; + attempted++; + try + { var unsubscribeUrl = BuildUnsubscribeUrl(subscriber.Email); var manageUrl = BuildManageUrl(subscriber.Email, _options.UnsubscribeSecret); var subject = relevantRoundups.Count == 1 @@ -146,20 +142,19 @@ public async Task SendCombinedWeeklyAsync(IReadOnlyList roundupSlu successful++; } } - } - catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) - { - var status = successful > 0 ? "partial" : "failed"; - await _subscriberRepository.LogSendAsync("weekly-roundup", targetKey, successful, status, ex.Message, ct); - _logger.LogError(ex, "Failed sending combined weekly newsletter"); - return false; +#pragma warning disable CA1031 // Best-effort: continue with other subscribers if one fails + catch (Exception ex) when (ex is not OperationCanceledException) + { + _logger.LogWarning(ex, "Failed sending weekly newsletter to subscriber β€” skipping"); + } +#pragma warning restore CA1031 } var sendStatus = attempted == 0 || successful == attempted ? "sent" : successful > 0 ? "partial" : "failed"; var sendError = attempted == 0 || successful == attempted ? null : $"Delivered to {successful} of {attempted} subscribers"; - await _subscriberRepository.LogSendAsync("weekly-roundup", targetKey, successful, sendStatus, sendError, ct); + await _subscriberRepository.LogSendAsync("weekly-roundup", targetKey, successful, attempted - successful, sendStatus, sendError, ct); return attempted == 0 || successful > 0; } @@ -225,8 +220,9 @@ await _subscriberRepository.LogSendAsync( "test-send", logKey, sent ? 1 : 0, + sent ? 0 : 1, sent ? "sent" : "failed", - sent ? null : $"Delivery failed to {recipientEmail}", + sent ? null : "Delivery failed", ct); return sent; } @@ -272,8 +268,9 @@ await _subscriberRepository.LogSendAsync( "test-daily", logKey, sent ? 1 : 0, + sent ? 0 : 1, sent ? "sent" : "failed", - sent ? null : $"Delivery failed to {recipientEmail}", + sent ? null : "Delivery failed", ct); return sent; } @@ -297,14 +294,11 @@ public async Task SendManageLinkEmailAsync(string email, CancellationToken } var manageUrl = BuildManageUrl(email, secret); - var html = $""" - -

Manage your TechHub newsletter subscription

-

Click the button below to manage your subscription preferences.

-

Manage subscription

-

Or copy this link into your browser:
{WebUtility.HtmlEncode(manageUrl)}

- - """; + var html = BuildAccountActionHtml( + title: "Manage your TechHub newsletter subscription", + message: "Click the button below to manage your subscription preferences.", + actionLabel: "Manage subscription", + actionUrl: manageUrl); var text = $""" Manage your TechHub newsletter subscription @@ -393,7 +387,7 @@ public async Task SendDailyOverviewAsync(DateOnly day, CancellationToken c if (dailySubscribers.Count == 0) { - await _subscriberRepository.LogSendAsync(SendKind, targetKey, 0, "sent", null, ct); + await _subscriberRepository.LogSendAsync(SendKind, targetKey, 0, 0, "sent", null, ct); return true; } @@ -427,7 +421,7 @@ public async Task SendDailyOverviewAsync(DateOnly day, CancellationToken c : sent > 0 ? $"Delivered to {sent} of {eligible} eligible subscribers" : "Delivery failed for all eligible subscribers"; - await _subscriberRepository.LogSendAsync(SendKind, targetKey, sent, sendStatus, sendError, ct); + await _subscriberRepository.LogSendAsync(SendKind, targetKey, sent, eligible - sent, sendStatus, sendError, ct); return sent > 0; } @@ -450,7 +444,7 @@ public async Task SendAdminStatusReportAsync(DateOnly day, CancellationTok var text = BuildAdminStatusText(day, stats); var sent = await SendEmailAsync(_options.AdminReportEmailAddress, $"TechHub Daily Status Report β€” {targetKey}", html, text, ct); - await _subscriberRepository.LogSendAsync(SendKind, targetKey, sent ? 1 : 0, sent ? "sent" : "failed", sent ? null : "Unable to send admin status report", ct); + await _subscriberRepository.LogSendAsync(SendKind, targetKey, sent ? 1 : 0, sent ? 0 : 1, sent ? "sent" : "failed", sent ? null : "Unable to send admin status report", ct); return sent; } @@ -465,20 +459,11 @@ public async Task SendConfirmationEmailAsync(string email, CancellationTok } var confirmUrl = BuildConfirmUrl(email); - var html = $""" - - - - - - - -

Confirm your TechHub newsletter subscription

-

Click the button below to confirm your subscription. If you did not sign up, you can safely ignore this email.

-

Confirm subscription

-

Or copy this link into your browser:
{WebUtility.HtmlEncode(confirmUrl)}

- - """; + var html = BuildAccountActionHtml( + title: "Confirm your TechHub newsletter subscription", + message: "Click the button below to confirm your subscription. If you did not sign up, you can safely ignore this email.", + actionLabel: "Confirm subscription", + actionUrl: confirmUrl); var text = $""" Confirm your TechHub newsletter subscription @@ -682,66 +667,43 @@ FROM ranked return result; } - private string BuildSectionLinksHtml(RoundupRow roundup) + private IEnumerable GetRoundupLinks(RoundupRow roundup) { - var headers = roundup.Content + var baseRoundupUrl = BuildAbsoluteUrl($"/{roundup.SectionName}/roundups/{roundup.Slug}"); + return roundup.Content .Split('\n', StringSplitOptions.RemoveEmptyEntries) .Where(line => line.StartsWith("## ", StringComparison.Ordinal)) .Select(line => line[3..].Trim()) .Distinct(StringComparer.OrdinalIgnoreCase) - .ToList(); - - var baseRoundupUrl = BuildAbsoluteUrl($"/{roundup.SectionName}/roundups/{roundup.Slug}"); - var sb = new StringBuilder(); - sb.Append(""); - return sb.ToString(); + .Select(header => (object)new + { + href = $"{baseRoundupUrl}#{RoundupContentBuilder.BuildAnchor(header)}", + text = header + }); } private string RenderCombinedWeeklyHtml(IReadOnlyList roundups, string unsubscribeUrl, string manageUrl) { - var sb = new StringBuilder(); - - for (var i = 0; i < roundups.Count; i++) + var footer = _templates.SubscriberFooter(new { manageUrl, unsubscribeUrl }); + var content = _templates.WeeklyContent(new { - var roundup = roundups[i]; - var sectionTitle = GetSectionTitle(roundup.SectionName); - var fullRoundupUrl = BuildAbsoluteUrl($"/{roundup.SectionName}/roundups/{roundup.Slug}"); - var sectionLinksHtml = BuildSectionLinksHtml(roundup); - - if (i > 0) + roundups = roundups.Select(r => new { - sb.Append("
"); - } - - sb.Append(CultureInfo.InvariantCulture, $""" -
-

{WebUtility.HtmlEncode(sectionTitle)}

-

{WebUtility.HtmlEncode(roundup.Title)}

-

{WebUtility.HtmlEncode(roundup.Introduction)}

-

πŸ“‘ In this roundup

-
{sectionLinksHtml}
-

- Read the full {WebUtility.HtmlEncode(sectionTitle)} roundup β†’ -

-
- """); - } - - return _weeklyDigestTemplate - .Replace("{Content}", sb.ToString(), StringComparison.Ordinal) - .Replace("{ManageUrl}", WebUtility.HtmlEncode(manageUrl), StringComparison.Ordinal) - .Replace("{UnsubscribeUrl}", WebUtility.HtmlEncode(unsubscribeUrl), StringComparison.Ordinal); + sectionTitle = GetSectionTitle(r.SectionName), + title = StripRoundupPrefix(r.Title), + introduction = r.Introduction, + roundupUrl = BuildAbsoluteUrl($"/{r.SectionName}/roundups/{r.Slug}"), + links = GetRoundupLinks(r) + }), + footer + }); + return _templates.Shell(new + { + title = "TechHub Weekly Digest", + cardClass = "th-card--weekly", + maxWidth = "900px", + content + }); } private string BuildCombinedWeeklyPlainText(IReadOnlyList roundups, string unsubscribeUrl, string manageUrl) @@ -757,7 +719,7 @@ private string BuildCombinedWeeklyPlainText(IReadOnlyList roundups, var fullRoundupUrl = BuildAbsoluteUrl($"/{roundup.SectionName}/roundups/{roundup.Slug}"); sb.AppendLine(sectionTitle); sb.AppendLine(new string('-', sectionTitle.Length)); - sb.AppendLine(roundup.Title); + sb.AppendLine(StripRoundupPrefix(roundup.Title)); sb.AppendLine(); sb.AppendLine(roundup.Introduction); sb.AppendLine(); @@ -774,6 +736,12 @@ private string BuildCombinedWeeklyPlainText(IReadOnlyList roundups, private string GetSectionTitle(string slug) => _appSettings.Content.Sections.TryGetValue(slug, out var config) ? config.Title : slug; + private static string StripRoundupPrefix(string title) + { + var colonIndex = title.IndexOf(": ", StringComparison.Ordinal); + return colonIndex >= 0 ? title[(colonIndex + 2)..] : title; + } + private List OrderSectionsByWebsiteOrder(IEnumerable sectionNames) { var sectionOrder = _appSettings.Content.Sections @@ -813,35 +781,31 @@ private string BuildDailyOverviewHtml( { var unsubscribeUrl = BuildUnsubscribeUrl(email); var manageUrl = BuildManageUrl(email, _options.UnsubscribeSecret); - - var sb = new StringBuilder(); - foreach (var section in sections) + var footer = _templates.SubscriberFooter(new { manageUrl, unsubscribeUrl }); + var content = _templates.DailyContent(new { - var items = itemsBySection.TryGetValue(section, out var rows) ? rows : []; - if (items.Count == 0) - { - continue; // Section has no news today β€” skip it - } - - sb.Append($"

{WebUtility.HtmlEncode(GetSectionTitle(section))}

"); - sb.Append("
    "); - foreach (var item in items) - { - var filteredUrl = BuildAbsoluteUrl($"/{section}/all?search={Uri.EscapeDataString(item.Title)}&exact=true"); - sb.Append("
  • "); - sb.Append($"{WebUtility.HtmlEncode(item.Title)}"); - sb.Append($" ({WebUtility.HtmlEncode(item.CollectionName)})"); - sb.Append("
    • "); - } - - sb.Append("
"); - } - - return _dailyOverviewTemplate - .Replace("{Date}", day.ToString("yyyy-MM-dd", CultureInfo.InvariantCulture), StringComparison.Ordinal) - .Replace("{Content}", sb.ToString(), StringComparison.Ordinal) - .Replace("{ManageUrl}", WebUtility.HtmlEncode(manageUrl), StringComparison.Ordinal) - .Replace("{UnsubscribeUrl}", WebUtility.HtmlEncode(unsubscribeUrl), StringComparison.Ordinal); + date = day.ToString("yyyy-MM-dd", CultureInfo.InvariantCulture), + sections = sections + .Where(s => itemsBySection.TryGetValue(s, out var rows) && rows.Count > 0) + .Select(s => new + { + title = GetSectionTitle(s), + items = itemsBySection[s].Select(item => new + { + url = BuildAbsoluteUrl($"/{s}/all?search={Uri.EscapeDataString(item.Title)}&exact=true"), + title = item.Title, + collectionName = item.CollectionName + }) + }), + footer + }); + return _templates.Shell(new + { + title = "TechHub Daily Overview", + cardClass = "th-card--daily", + maxWidth = "1100px", + content + }); } private string BuildDailyOverviewText( @@ -878,15 +842,38 @@ private string BuildDailyOverviewText( return sb.ToString(); } - private string BuildAdminStatusHtml(DateOnly day, NewsletterDailyReportStats stats) => - _adminStatusTemplate - .Replace("{Date}", day.ToString("yyyy-MM-dd", CultureInfo.InvariantCulture), StringComparison.Ordinal) - .Replace("{NewContentItems}", stats.NewContentItemsLast24Hours.ToString(CultureInfo.InvariantCulture), StringComparison.Ordinal) - .Replace("{FailedUrls}", stats.FailedProcessedUrlsLast24Hours.ToString(CultureInfo.InvariantCulture), StringComparison.Ordinal) - .Replace("{FailedJobs}", stats.FailedJobsLast24Hours.ToString(CultureInfo.InvariantCulture), StringComparison.Ordinal) - .Replace("{NewSubscribers}", stats.NewSubscribersLast24Hours.ToString(CultureInfo.InvariantCulture), StringComparison.Ordinal) - .Replace("{ActiveSubscribers}", stats.ActiveSubscribers.ToString(CultureInfo.InvariantCulture), StringComparison.Ordinal) - .Replace("{UnconfirmedSubscribers}", stats.UnconfirmedSubscribers.ToString(CultureInfo.InvariantCulture), StringComparison.Ordinal); + private string BuildAdminStatusHtml(DateOnly day, NewsletterDailyReportStats stats) + { + var content = _templates.AdminContent(new + { + newContentItems = stats.NewContentItemsLast24Hours.ToString(CultureInfo.InvariantCulture), + failedProcessedUrls = stats.FailedProcessedUrlsLast24Hours.ToString(CultureInfo.InvariantCulture), + failedJobs = stats.FailedJobsLast24Hours.ToString(CultureInfo.InvariantCulture), + failedNewsletterSends = stats.FailedNewsletterSendsLast24Hours.ToString(CultureInfo.InvariantCulture), + newSubscribers = stats.NewSubscribersLast24Hours.ToString(CultureInfo.InvariantCulture), + activeSubscribers = stats.ActiveSubscribers.ToString(CultureInfo.InvariantCulture), + unconfirmedSubscribers = stats.UnconfirmedSubscribers.ToString(CultureInfo.InvariantCulture) + }); + return _templates.Shell(new + { + title = $"TechHub Daily Status Report \u2014 {day.ToString("yyyy-MM-dd", CultureInfo.InvariantCulture)}", + cardClass = "th-card--admin", + maxWidth = "600px", + content + }); + } + + private string BuildAccountActionHtml(string title, string message, string actionLabel, string actionUrl) + { + var content = _templates.AccountAction(new { message, actionLabel, actionUrl }); + return _templates.Shell(new + { + title, + cardClass = "th-card--account", + maxWidth = "640px", + content + }); + } private static string BuildAdminStatusText(DateOnly day, NewsletterDailyReportStats stats) => string.Create(CultureInfo.InvariantCulture, $""" @@ -895,6 +882,7 @@ private static string BuildAdminStatusText(DateOnly day, NewsletterDailyReportSt New content items (24h): {stats.NewContentItemsLast24Hours} Failed processed URLs (24h): {stats.FailedProcessedUrlsLast24Hours} Failed background jobs (24h): {stats.FailedJobsLast24Hours} + Failed newsletter sends (24h): {stats.FailedNewsletterSendsLast24Hours} New newsletter subscribers (24h): {stats.NewSubscribersLast24Hours} Active subscribers: {stats.ActiveSubscribers} Unconfirmed subscribers: {stats.UnconfirmedSubscribers} @@ -941,19 +929,6 @@ private bool IsUnsubscribeSecretConfigured(string operationName) return false; } - private static string LoadEmbeddedHtml(string resourceName, string fallback = "") - { - var assembly = Assembly.GetExecutingAssembly(); - using var stream = assembly.GetManifestResourceStream(resourceName); - if (stream is null) - { - return fallback; - } - - using var reader = new StreamReader(stream); - return reader.ReadToEnd(); - } - private sealed class RoundupRow { public string Slug { get; init; } = string.Empty; diff --git a/src/TechHub.Infrastructure/Services/Newsletter/NewsletterTemplateProvider.cs b/src/TechHub.Infrastructure/Services/Newsletter/NewsletterTemplateProvider.cs new file mode 100644 index 000000000..39a5dcf18 --- /dev/null +++ b/src/TechHub.Infrastructure/Services/Newsletter/NewsletterTemplateProvider.cs @@ -0,0 +1,54 @@ +using System.Reflection; +using HandlebarsDotNet; +using Microsoft.Extensions.Hosting; + +namespace TechHub.Infrastructure.Services.Newsletter; + +/// +/// Singleton service that compiles Handlebars email templates once at startup. +/// Injected into so templates are not recompiled +/// per DI scope. +/// +public sealed class NewsletterTemplateProvider +{ + public HandlebarsTemplate Shell { get; } + public HandlebarsTemplate SubscriberFooter { get; } + public HandlebarsTemplate AccountAction { get; } + public HandlebarsTemplate WeeklyContent { get; } + public HandlebarsTemplate DailyContent { get; } + public HandlebarsTemplate AdminContent { get; } + + public NewsletterTemplateProvider(IHostEnvironment env) + { + ArgumentNullException.ThrowIfNull(env); + + if (env.IsDevelopment()) + { + var resourcesPath = Path.GetFullPath(Path.Join(env.ContentRootPath, "..", "TechHub.Infrastructure", "Data", "Resources")); + Shell = Handlebars.Compile(File.ReadAllText(Path.Join(resourcesPath, "newsletter-template.html"))); + SubscriberFooter = Handlebars.Compile(File.ReadAllText(Path.Join(resourcesPath, "newsletter-subscriber-footer.html"))); + AccountAction = Handlebars.Compile(File.ReadAllText(Path.Join(resourcesPath, "newsletter-account-action-content.html"))); + WeeklyContent = Handlebars.Compile(File.ReadAllText(Path.Join(resourcesPath, "newsletter-weekly-content.html"))); + DailyContent = Handlebars.Compile(File.ReadAllText(Path.Join(resourcesPath, "newsletter-daily-content.html"))); + AdminContent = Handlebars.Compile(File.ReadAllText(Path.Join(resourcesPath, "newsletter-admin-content.html"))); + } + else + { + Shell = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-template.html")); + SubscriberFooter = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-subscriber-footer.html")); + AccountAction = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-account-action-content.html")); + WeeklyContent = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-weekly-content.html")); + DailyContent = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-daily-content.html")); + AdminContent = Handlebars.Compile(LoadEmbeddedHtml("TechHub.Infrastructure.Data.Resources.newsletter-admin-content.html")); + } + } + + private static string LoadEmbeddedHtml(string resourceName) + { + var assembly = Assembly.GetExecutingAssembly(); + using var stream = assembly.GetManifestResourceStream(resourceName) + ?? throw new InvalidOperationException($"Embedded resource '{resourceName}' not found."); + using var reader = new StreamReader(stream); + return reader.ReadToEnd(); + } +} diff --git a/src/TechHub.Infrastructure/TechHub.Infrastructure.csproj b/src/TechHub.Infrastructure/TechHub.Infrastructure.csproj index 56eb79816..065587791 100644 --- a/src/TechHub.Infrastructure/TechHub.Infrastructure.csproj +++ b/src/TechHub.Infrastructure/TechHub.Infrastructure.csproj @@ -14,6 +14,7 @@ + @@ -37,9 +38,12 @@ - - - + + + + + + diff --git a/src/TechHub.ServiceDefaults/Extensions.cs b/src/TechHub.ServiceDefaults/Extensions.cs index 9664017b9..410901411 100644 --- a/src/TechHub.ServiceDefaults/Extensions.cs +++ b/src/TechHub.ServiceDefaults/Extensions.cs @@ -39,14 +39,20 @@ public static class ServiceDefaultsExtensions /// Use this to enrich or suppress activities based on response properties /// (e.g. clear ActivityTraceFlags.Recorded for 4xx responses). /// + /// + /// Optional added to the tracing pipeline. + /// Use this to suppress or enrich activities that bypass the HTTP filter (e.g. SignalR + /// hub method invocations that have no ). + /// public static IHostApplicationBuilder AddServiceDefaults( this IHostApplicationBuilder builder, Func? additionalTraceFilter = null, - Action? additionalResponseEnricher = null) + Action? additionalResponseEnricher = null, + BaseProcessor? additionalActivityProcessor = null) { ArgumentNullException.ThrowIfNull(builder); - builder.ConfigureOpenTelemetry(additionalTraceFilter, additionalResponseEnricher); + builder.ConfigureOpenTelemetry(additionalTraceFilter, additionalResponseEnricher, additionalActivityProcessor); builder.AddDefaultHealthChecks(); @@ -78,10 +84,16 @@ public static IHostApplicationBuilder AddServiceDefaults( /// Optional callback invoked by EnrichWithHttpResponse after the response is written. /// Use this to enrich or suppress activities based on response properties. /// + /// + /// Optional added to the tracing pipeline. + /// Use this to suppress or enrich activities that bypass the HTTP filter (e.g. SignalR + /// hub method invocations that have no ). + /// public static IHostApplicationBuilder ConfigureOpenTelemetry( this IHostApplicationBuilder builder, Func? additionalTraceFilter = null, - Action? additionalResponseEnricher = null) + Action? additionalResponseEnricher = null, + BaseProcessor? additionalActivityProcessor = null) { ArgumentNullException.ThrowIfNull(builder); @@ -153,6 +165,11 @@ public static IHostApplicationBuilder ConfigureOpenTelemetry( }; }) .AddHttpClientInstrumentation(); + + if (additionalActivityProcessor != null) + { + tracing.AddProcessor(additionalActivityProcessor); + } }); builder.AddOpenTelemetryExporters(); diff --git a/src/TechHub.Web/Components/Layout/MainLayout.razor b/src/TechHub.Web/Components/Layout/MainLayout.razor index 868883ed6..bf1320b56 100644 --- a/src/TechHub.Web/Components/Layout/MainLayout.razor +++ b/src/TechHub.Web/Components/Layout/MainLayout.razor @@ -34,7 +34,7 @@ else diff --git a/src/TechHub.Web/Components/Pages/About.razor b/src/TechHub.Web/Components/Pages/About.razor index 46ae82176..257546776 100644 --- a/src/TechHub.Web/Components/Pages/About.razor +++ b/src/TechHub.Web/Components/Pages/About.razor @@ -77,28 +77,6 @@ - -
- Fokko Veegens -

Fokko Veegens

-

DevOps Consultant

-

- Fokko is a DevOps Consultant specializing in Azure, GitHub, and modern software - development practices. He helps organizations optimize their development workflows - and adopt cloud-native technologies effectively. -

-
- - πŸ“˜ GitHub - - - πŸ’Ό LinkedIn - - -
-

Why did we build this

diff --git a/src/TechHub.Web/Components/Pages/Admin/AdminNewsletter.razor b/src/TechHub.Web/Components/Pages/Admin/AdminNewsletter.razor index 822210d19..aef8d575d 100644 --- a/src/TechHub.Web/Components/Pages/Admin/AdminNewsletter.razor +++ b/src/TechHub.Web/Components/Pages/Admin/AdminNewsletter.razor @@ -118,7 +118,8 @@ When Kind Target - Recipients + Sent + Failed Status @@ -130,6 +131,7 @@ @log.SendKind @log.TargetKey @log.RecipientCount + @(log.FailedCount > 0 ? log.FailedCount.ToString() : "β€”") @log.Status } diff --git a/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs b/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs new file mode 100644 index 000000000..b6559a998 --- /dev/null +++ b/src/TechHub.Web/Middleware/HeadRequestMiddleware.cs @@ -0,0 +1,149 @@ +using TechHub.Core.Validation; + +namespace TechHub.Web.Middleware; + +/// +/// Middleware that handles HEAD requests without engaging the Blazor SSR pipeline. +/// +/// +/// Two categories of HEAD request are handled differently: +/// +/// +/// Extension-less Blazor page routes β€” Returned immediately with +/// 200 OK and Content-Type: text/html, bypassing Blazor SSR entirely. +/// The SSR pipeline makes an API call for every page render; when the API is slow, +/// bots and crawlers (Slack link previews, SEO scanners) disconnect after their +/// 15–25 s timeout and record 499 (client closed request) in telemetry. Since HEAD +/// only requires response headers β€” not a body β€” returning 200 immediately avoids +/// these 499s; note that for unknown routes the response may not mirror a GET (which +/// would 404 after SSR slug validation), so this is a deliberate performance +/// trade-off rather than a strict semantic guarantee. +/// Known minimal API endpoints (/version, /health, /alive) +/// are excluded from this path and fall through to the rewrite path below. +/// File-extension paths and minimal API endpoints β€” Rewrites the method to +/// GET, sets Response.Body = Stream.Null to suppress body output, and calls +/// the next middleware. MapStaticAssets serves the correct headers (ETag, +/// Content-Length, Cache-Control) without sending bytes; MapGet endpoints +/// (RSS feeds, /version, /health, /alive, etc.) also work correctly this way. +/// +/// +/// +/// This middleware must be placed immediately before UseRouting() so that the method +/// rewrite (HEAD β†’ GET) is visible to endpoint selection. UseInvalidRouteSegmentFilter +/// must run before this middleware so that invalid paths are already rejected with 404. +/// +/// +public class HeadRequestMiddleware +{ + // Exact extension-less endpoints that must NOT be short-circuited. + // They are not Blazor page routes so they must go through the HEADβ†’GET rewrite + // path so their handlers can run and return correct headers/status codes. + private static readonly HashSet _excludedExactPaths = new(StringComparer.OrdinalIgnoreCase) + { + "/version", + "/health", + "/alive", + "/signin-oidc", + "/admin/logout", + }; + + // Prefix-based endpoints that must NOT be short-circuited. + private static readonly string[] _excludedPrefixes = + [ + "/_blazor", + "/MicrosoftIdentity", + ]; + + private readonly RequestDelegate _next; + + public HeadRequestMiddleware(RequestDelegate next) + { + ArgumentNullException.ThrowIfNull(next); + _next = next; + } + + public async Task InvokeAsync(HttpContext context) + { + ArgumentNullException.ThrowIfNull(context); + + if (!HttpMethods.IsHead(context.Request.Method)) + { + await _next(context); + return; + } + + // Determine whether this is an extension-less path (Blazor page route) or a + // file-extension path (static asset, RSS feed, etc.) by inspecting the last segment. + var path = context.Request.Path.Value ?? "/"; + var lastSlash = path.LastIndexOf('/'); + var lastSegment = path.AsSpan()[(lastSlash + 1)..]; + var isExtensionless = !lastSegment.Contains('.'); + + if (isExtensionless && !IsShortCircuitExcluded(context.Request.Path)) + { + // Validate all path segments structurally before short-circuiting. + // InvalidRouteSegmentMiddleware validates only the first segment; segments + // beyond that (collection, slug, date-component, etc.) are checked here so + // that clearly invalid paths return 404 rather than a misleading 200. + // IsValidSlug covers all structurally valid URL segments (letters, digits, + // hyphens) and rejects dots, spaces, special characters, and other patterns + // that can never match a Blazor route. + var segments = path.Split('/', StringSplitOptions.RemoveEmptyEntries); + if (segments.Any(segment => !RouteParameterValidator.IsValidSlug(segment))) + { + context.Response.StatusCode = StatusCodes.Status404NotFound; + return; + } + + // Extension-less path β†’ Blazor page route. Short-circuit with 200 immediately. + // Calling next would trigger Blazor SSR β†’ API call β†’ potential slow response β†’ + // bot/crawler timeout β†’ 499. HEAD only needs headers, not a body. + context.Response.StatusCode = StatusCodes.Status200OK; + context.Response.ContentType = "text/html; charset=utf-8"; + return; + } + + // File-extension path: rewrite HEAD β†’ GET so endpoint selection (which registers + // only GET handlers) can match. Stream.Null suppresses body output as HEAD requires. + context.Request.Method = HttpMethods.Get; + var originalBody = context.Response.Body; + context.Response.Body = Stream.Null; + try + { + await _next(context); + } + finally + { + context.Response.Body = originalBody; + context.Request.Method = HttpMethods.Head; + } + } + + private static bool IsShortCircuitExcluded(PathString path) + { + var value = path.Value; + if (value is not null && _excludedExactPaths.Contains(value)) + { + return true; + } + + for (var i = 0; i < _excludedPrefixes.Length; i++) + { + if (path.StartsWithSegments(_excludedPrefixes[i], StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + + return false; + } +} + +/// +/// Extension method for registering . +/// +public static class HeadRequestMiddlewareExtensions +{ + public static IApplicationBuilder UseHeadRequestHandling(this IApplicationBuilder builder) + => builder.UseMiddleware(); +} diff --git a/src/TechHub.Web/Program.cs b/src/TechHub.Web/Program.cs index a2bc74bc6..b059678d8 100644 --- a/src/TechHub.Web/Program.cs +++ b/src/TechHub.Web/Program.cs @@ -25,9 +25,15 @@ // WebTelemetryFilters suppresses Blazor disconnect noise and bot-crawler 404s from traces. // SuppressIfClientError clears ActivityTraceFlags.Recorded for 4xx responses so they are // not exported to App Insights and do not inflate the requests/failed metric. +// additionalActivityProcessor: suppresses failed ComponentHub/UpdateRootComponents activities +// (Blazor Server push to disconnected circuits, success=false resultCode=0 in App Insights). +// CA2000 suppressed: TracerProviderBuilder.AddProcessor takes ownership and disposes the processor. +#pragma warning disable CA2000 // Dispose objects before losing scope builder.AddServiceDefaults( additionalTraceFilter: WebTelemetryFilters.ShouldTrace, - additionalResponseEnricher: WebTelemetryFilters.SuppressIfClientError); + additionalResponseEnricher: WebTelemetryFilters.SuppressIfClientError, + additionalActivityProcessor: new BlazorHubNoiseSuppressor()); +#pragma warning restore CA2000 // Dispose objects before losing scope // Log environment during startup for verification using (var loggerFactory = LoggerFactory.Create(b => b.AddConsole())) @@ -461,34 +467,15 @@ // reach the auth stack or endpoint selection. app.UseInvalidRouteSegmentFilter(); -// ── HEAD β†’ GET rewrite (must be immediately before UseRouting) ──────────────── -// MapRazorComponents and MapGet only register GET endpoints. In WebApplication, -// UseRouting() is auto-inserted at the start of the pipeline (before user middleware), -// so it sees the original HEAD method and returns 405. To fix this, we place an -// explicit app.UseRouting() AFTER the rewrite so routing happens after the method -// is already GET. The body is suppressed via Stream.Null so HEAD responses are empty. -app.Use(async (context, next) => -{ - if (HttpMethods.IsHead(context.Request.Method)) - { - context.Request.Method = HttpMethods.Get; - var originalBody = context.Response.Body; - context.Response.Body = Stream.Null; - try - { - await next(); - } - finally - { - context.Response.Body = originalBody; - context.Request.Method = HttpMethods.Head; - } - } - else - { - await next(); - } -}); +// ── HEAD request handling (must be immediately before UseRouting) ─────────────── +// Extension-less HEAD requests (Blazor page routes) are short-circuited with 200 OK +// before Blazor SSR runs. SSR triggers an API call; when the API is slow, bots and +// crawlers disconnect after their timeout and generate 499s with ~23 s duration. +// File-extension HEAD requests (static assets, RSS) are rewritten to GET with +// Stream.Null to suppress the body β€” MapStaticAssets then serves correct headers. +// UseRouting() is placed explicitly after this middleware so endpoint selection sees +// GET rather than HEAD (MapRazorComponents only registers GET endpoints). +app.UseHeadRequestHandling(); // Explicit UseRouting() placement: must come AFTER the HEADβ†’GET rewrite so that // endpoint selection (including HTTP-method matching) sees GET rather than HEAD. diff --git a/src/TechHub.Web/Telemetry/BlazorHubNoiseSuppressor.cs b/src/TechHub.Web/Telemetry/BlazorHubNoiseSuppressor.cs new file mode 100644 index 000000000..c48935f11 --- /dev/null +++ b/src/TechHub.Web/Telemetry/BlazorHubNoiseSuppressor.cs @@ -0,0 +1,48 @@ +using System.Diagnostics; +using OpenTelemetry; + +namespace TechHub.Web.Telemetry; + +/// +/// OpenTelemetry activity processor that suppresses failed Blazor Server hub method +/// activities from the trace export pipeline. +/// +/// +/// When the client has already disconnected (navigated away, closed the tab), the server still +/// attempts to push a component update, which fails because the circuit is gone. This produces +/// a ComponentHub/UpdateRootComponents activity with +/// β€” no HTTP status code, so Application Insights +/// records it as success = false, resultCode = 0. These are structurally expected and +/// have no diagnostic value; they are a normal side-effect of Blazor Server's server-driven +/// rendering model. +/// +/// +/// +/// Successful hub invocations (circuit still connected, update delivered) are left untouched +/// so they remain visible in Application Insights for performance analysis. +/// +/// +/// +/// This processor clears in OnEnd only for +/// error-status activities, so the Azure Monitor BatchExportProcessor does not queue +/// them for export. Consistent with the approach in +/// for 404/405 HTTP responses. +/// +/// +internal sealed class BlazorHubNoiseSuppressor : BaseProcessor +{ + // All Blazor Server circuit hub invocations share this prefix in their display name. + // Examples: + // Microsoft.AspNetCore.Components.Server.ComponentHub/UpdateRootComponents + // Microsoft.AspNetCore.Components.Server.ComponentHub/BeginInvokeDotNetFromJS + private const string ComponentHubPrefix = "Microsoft.AspNetCore.Components.Server.ComponentHub"; + + public override void OnEnd(Activity activity) + { + if (activity.DisplayName.StartsWith(ComponentHubPrefix, StringComparison.Ordinal) + && activity.Status == ActivityStatusCode.Error) + { + activity.ActivityTraceFlags &= ~ActivityTraceFlags.Recorded; + } + } +} diff --git a/tests/TechHub.E2E.Tests/Web/AboutPageTests.cs b/tests/TechHub.E2E.Tests/Web/AboutPageTests.cs index 9662b4081..a8d915c3f 100644 --- a/tests/TechHub.E2E.Tests/Web/AboutPageTests.cs +++ b/tests/TechHub.E2E.Tests/Web/AboutPageTests.cs @@ -28,7 +28,6 @@ public async Task AboutPage_ShouldDisplay_TeamMembers() // Assert - Check for team member headings await Page.AssertElementVisibleByRoleAsync(AriaRole.Heading, "Reinier van Maanen"); await Page.AssertElementVisibleByRoleAsync(AriaRole.Heading, "Rob Bos"); - await Page.AssertElementVisibleByRoleAsync(AriaRole.Heading, "Fokko Veegens"); } [Fact] @@ -40,7 +39,6 @@ public async Task AboutPage_ShouldDisplay_TeamMemberPhotos() // Assert - Check for team member photos (exact alt text matching) await Page.AssertElementVisibleByAltTextAsync("Reinier van Maanen"); await Page.AssertElementVisibleByAltTextAsync("Rob Bos"); - await Page.AssertElementVisibleByAltTextAsync("Fokko Veegens"); } [Fact] @@ -62,14 +60,6 @@ public async Task AboutPage_ShouldDisplay_SocialLinks() await robSection.AssertElementVisibleByRoleAsync(AriaRole.Link, "πŸ“˜ GitHub"); await robSection.AssertElementVisibleByRoleAsync(AriaRole.Link, "πŸ’Ό LinkedIn"); await robSection.AssertElementVisibleByRoleAsync(AriaRole.Link, "✍️ Blog"); - - // Fokko Veegens - should have GitHub and LinkedIn - var fokkoSection = Page.Locator("text=Fokko Veegens").Locator(".."); - await fokkoSection.AssertElementVisibleByRoleAsync(AriaRole.Link, "πŸ“˜ GitHub"); - await fokkoSection.AssertElementVisibleByRoleAsync(AriaRole.Link, "πŸ’Ό LinkedIn"); - - // Note: Fokko's blog link might not be visible or might be a placeholder - // This test verifies the presence of GitHub and LinkedIn only } [Fact] diff --git a/tests/TechHub.E2E.Tests/Web/GitHubCopilotFeaturesTests.cs b/tests/TechHub.E2E.Tests/Web/GitHubCopilotFeaturesTests.cs index 22ac171fa..3c5c24442 100644 --- a/tests/TechHub.E2E.Tests/Web/GitHubCopilotFeaturesTests.cs +++ b/tests/TechHub.E2E.Tests/Web/GitHubCopilotFeaturesTests.cs @@ -89,9 +89,11 @@ public async Task GitHubCopilotFeatures_PaidTiers_ShouldBeStacked_InSidebar() // Assert - All tiers should be stacked vertically in the sidebar var sidebar = Page.Locator("aside.sidebar"); + var tiersSection = sidebar.Locator(".features-tiers-sidebar"); + await tiersSection.AssertElementVisibleAsync(); + var tierCards = sidebar.Locator(".features-tier-card"); - var count = await tierCards.CountAsync(); - count.Should().Be(6, "Expected 6 tier cards stacked in the sidebar"); + await tierCards.AssertCountAsync(6); } [Fact] diff --git a/tests/TechHub.Infrastructure.Tests/Repositories/NewsletterSubscriberRepositoryTests.cs b/tests/TechHub.Infrastructure.Tests/Repositories/NewsletterSubscriberRepositoryTests.cs index fa855e87c..eee7a63ac 100644 --- a/tests/TechHub.Infrastructure.Tests/Repositories/NewsletterSubscriberRepositoryTests.cs +++ b/tests/TechHub.Infrastructure.Tests/Repositories/NewsletterSubscriberRepositoryTests.cs @@ -109,7 +109,7 @@ await _sut.UpsertSubscriberAsync( [Fact] public async Task LogSendAsync_AndHasBeenSentAsync_WorkAsExpected() { - await _sut.LogSendAsync("weekly-roundup", "weekly-ai-roundup-2026-05-18", 12, "sent", null, TestContext.Current.CancellationToken); + await _sut.LogSendAsync("weekly-roundup", "weekly-ai-roundup-2026-05-18", 12, 0, "sent", null, TestContext.Current.CancellationToken); var hasBeenSent = await _sut.HasBeenSentAsync("weekly-roundup", "weekly-ai-roundup-2026-05-18", TestContext.Current.CancellationToken); hasBeenSent.Should().BeTrue(); @@ -126,6 +126,7 @@ public async Task GetDailyReportStatsAsync_ReturnsNonNegativeCounts() stats.NewContentItemsLast24Hours.Should().BeGreaterThanOrEqualTo(0); stats.FailedProcessedUrlsLast24Hours.Should().BeGreaterThanOrEqualTo(0); stats.FailedJobsLast24Hours.Should().BeGreaterThanOrEqualTo(0); + stats.FailedNewsletterSendsLast24Hours.Should().BeGreaterThanOrEqualTo(0); stats.NewSubscribersLast24Hours.Should().BeGreaterThanOrEqualTo(0); stats.ActiveSubscribers.Should().BeGreaterThanOrEqualTo(0); stats.UnconfirmedSubscribers.Should().BeGreaterThanOrEqualTo(0); diff --git a/tests/TechHub.Infrastructure.Tests/Services/NewsletterServiceTests.cs b/tests/TechHub.Infrastructure.Tests/Services/NewsletterServiceTests.cs index e04b6d99c..54ad5e362 100644 --- a/tests/TechHub.Infrastructure.Tests/Services/NewsletterServiceTests.cs +++ b/tests/TechHub.Infrastructure.Tests/Services/NewsletterServiceTests.cs @@ -1,5 +1,6 @@ using Dapper; using FluentAssertions; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Moq; @@ -346,6 +347,9 @@ private NewsletterService CreateService( } }); + var env = new Mock(); + env.Setup(e => e.EnvironmentName).Returns(Environments.Production); + return new NewsletterService( _fixture.Connection, new NewsletterSubscriberRepository(_fixture.Connection), @@ -353,7 +357,8 @@ private NewsletterService CreateService( options, appSettings, emailSender, - NullLogger.Instance); + NullLogger.Instance, + new NewsletterTemplateProvider(env.Object)); } private static Section CreateSection(string name) => diff --git a/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs b/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs new file mode 100644 index 000000000..7f6c29c5e --- /dev/null +++ b/tests/TechHub.Web.Tests/Middleware/HeadRequestMiddlewareTests.cs @@ -0,0 +1,236 @@ +using FluentAssertions; +using Microsoft.AspNetCore.Http; +using TechHub.Web.Middleware; + +namespace TechHub.Web.Tests.Middleware; + +public class HeadRequestMiddlewareTests +{ + // ─── Non-HEAD methods pass through unchanged ────────────────────────────── + + [Theory] + [InlineData("GET")] + [InlineData("POST")] + [InlineData("PUT")] + public async Task InvokeAsync_NonHeadMethod_CallsNextUnchanged(string method) + { + var (middleware, context, nextCalled) = CreateMiddleware(method, "/some/path"); + + await middleware.InvokeAsync(context); + + nextCalled().Should().BeTrue(); + context.Request.Method.Should().Be(method); + } + + // ─── Extension-less HEAD β†’ 200 OK, no SSR ──────────────────────────────── + + [Theory] + [InlineData("/")] + [InlineData("/ai")] + [InlineData("/ai/videos/my-article-slug")] + [InlineData("/all/authors/john-doe")] + [InlineData("/about")] + [InlineData("/admin/login")] + [InlineData("/news/2024/some-post")] + public async Task InvokeAsync_HeadExtensionlessPath_Returns200WithoutCallingNext(string path) + { + var (middleware, context, nextCalled) = CreateMiddleware("HEAD", path); + + await middleware.InvokeAsync(context); + + nextCalled().Should().BeFalse("SSR must be bypassed for extension-less HEAD requests"); + context.Response.StatusCode.Should().Be(StatusCodes.Status200OK); + context.Response.ContentType.Should().Be("text/html; charset=utf-8"); + } + + // ─── Known minimal API endpoints must use the HEADβ†’GET rewrite path ─────── + + [Theory] + [InlineData("/version")] + [InlineData("/health")] + [InlineData("/alive")] + public async Task InvokeAsync_HeadMinimalApiPath_CallsNextWithGetMethod(string path) + { + string? methodSeenByNext = null; + RequestDelegate next = ctx => + { + methodSeenByNext = ctx.Request.Method; + return Task.CompletedTask; + }; + var middleware = new HeadRequestMiddleware(next); + var context = new DefaultHttpContext(); + context.Request.Method = "HEAD"; + context.Request.Path = path; + + await middleware.InvokeAsync(context); + + methodSeenByNext.Should().Be(HttpMethods.Get, "minimal API endpoints must see GET so their MapGet handlers match"); + context.Request.Method.Should().Be("HEAD", "method must be restored to HEAD after next() returns"); + } + + [Theory] + [InlineData("/_blazor")] + [InlineData("/_blazor/negotiate")] + [InlineData("/MicrosoftIdentity/Account/SignIn")] + [InlineData("/signin-oidc")] + [InlineData("/admin/logout")] + public async Task InvokeAsync_HeadExcludedNonPagePath_CallsNextWithGetMethod(string path) + { + string? methodSeenByNext = null; + RequestDelegate next = ctx => + { + methodSeenByNext = ctx.Request.Method; + return Task.CompletedTask; + }; + var middleware = new HeadRequestMiddleware(next); + var context = new DefaultHttpContext(); + context.Request.Method = "HEAD"; + context.Request.Path = path; + + await middleware.InvokeAsync(context); + + methodSeenByNext.Should().Be(HttpMethods.Get, "excluded non-page endpoints must use the HEADβ†’GET rewrite path"); + context.Request.Method.Should().Be("HEAD", "method must be restored to HEAD after next() returns"); + } + + [Fact] + public async Task InvokeAsync_HeadExtensionlessPath_MethodRemainsHead() + { + var (middleware, context, _) = CreateMiddleware("HEAD", "/ai/videos/article"); + + await middleware.InvokeAsync(context); + + // Method must not be mutated β€” HEAD remains HEAD so callers see the correct method. + context.Request.Method.Should().Be("HEAD"); + } + + // ─── File-extension HEAD β†’ GET rewrite, body suppressed ────────────────── + + [Theory] + [InlineData("/favicon.ico")] + [InlineData("/robots.txt")] + [InlineData("/js/mobile-nav.abc123.js")] + [InlineData("/all/feed.xml")] + [InlineData("/sitemap.xml")] + [InlineData("/css/app.min.css")] + public async Task InvokeAsync_HeadFileExtensionPath_CallsNextWithGetMethod(string path) + { + string? methodSeenByNext = null; + RequestDelegate next = ctx => + { + methodSeenByNext = ctx.Request.Method; + return Task.CompletedTask; + }; + var middleware = new HeadRequestMiddleware(next); + var context = new DefaultHttpContext(); + context.Request.Method = "HEAD"; + context.Request.Path = path; + + await middleware.InvokeAsync(context); + + methodSeenByNext.Should().Be(HttpMethods.Get, "next must see GET so endpoint selection matches MapStaticAssets/MapGet"); + } + + [Theory] + [InlineData("/favicon.ico")] + [InlineData("/robots.txt")] + [InlineData("/js/app.abc.js")] + [InlineData("/all/feed.xml")] + public async Task InvokeAsync_HeadFileExtensionPath_MethodRestoredToHeadAfterNext(string path) + { + var (middleware, context, nextCalled) = CreateMiddleware("HEAD", path); + + await middleware.InvokeAsync(context); + + nextCalled().Should().BeTrue(); + context.Request.Method.Should().Be("HEAD", "method must be restored to HEAD after next() returns"); + } + + [Fact] + public async Task InvokeAsync_HeadFileExtensionPath_BodyIsStreamNullDuringNextAndRestoredAfter() + { + using var originalBody = new MemoryStream(); + Stream? bodySeenByNext = null; + + RequestDelegate next = ctx => + { + bodySeenByNext = ctx.Response.Body; + return Task.CompletedTask; + }; + var middleware = new HeadRequestMiddleware(next); + var context = new DefaultHttpContext(); + context.Request.Method = "HEAD"; + context.Request.Path = "/favicon.ico"; + context.Response.Body = originalBody; + + await middleware.InvokeAsync(context); + + bodySeenByNext.Should().BeSameAs(Stream.Null, "body must be Stream.Null while next executes so no bytes are written"); + context.Response.Body.Should().BeSameAs(originalBody, "original body must be restored after next() returns"); + } + + [Fact] + public async Task InvokeAsync_HeadFileExtensionPath_BodyRestoredEvenWhenNextThrows() + { + using var originalBody = new MemoryStream(); + + RequestDelegate next = _ => throw new InvalidOperationException("simulated error"); + var middleware = new HeadRequestMiddleware(next); + var context = new DefaultHttpContext(); + context.Request.Method = "HEAD"; + context.Request.Path = "/favicon.ico"; + context.Response.Body = originalBody; + + await middleware.Invoking(m => m.InvokeAsync(context)).Should().ThrowAsync(); + + context.Response.Body.Should().BeSameAs(originalBody, "body must be restored even when next() throws"); + context.Request.Method.Should().Be("HEAD", "method must be restored even when next() throws"); + } + + // ─── Structurally invalid extension-less paths β†’ 404, no SSR ───────────── + + [Theory] + [InlineData("/ai/invalid segment")] // space in segment + [InlineData("/section/../../etc/passwd")] // path traversal + [InlineData("/section/col%lection")] // percent + invalid chars + [InlineData("/section/has_underscore")] // underscore is not a valid slug character + [InlineData("/section/has.dot/slug")] // dot in non-terminal segment + public async Task InvokeAsync_HeadExtensionlessPath_InvalidSegment_Returns404WithoutCallingNext(string path) + { + var (middleware, context, nextCalled) = CreateMiddleware("HEAD", path); + + await middleware.InvokeAsync(context); + + nextCalled().Should().BeFalse("SSR must be bypassed for structurally invalid extension-less HEAD requests"); + context.Response.StatusCode.Should().Be(StatusCodes.Status404NotFound); + } + + // ─── Constructor guard ──────────────────────────────────────────────────── + + [Fact] + public void Constructor_NullNext_ThrowsArgumentNullException() + { + var act = () => new HeadRequestMiddleware(null!); + + act.Should().Throw().WithParameterName("next"); + } + + // ─── Helper ─────────────────────────────────────────────────────────────── + + private static (HeadRequestMiddleware middleware, HttpContext context, Func nextCalled) CreateMiddleware( + string method = "GET", + string path = "/") + { + var wasCalled = false; + RequestDelegate next = _ => + { + wasCalled = true; + return Task.CompletedTask; + }; + var middleware = new HeadRequestMiddleware(next); + var context = new DefaultHttpContext(); + context.Request.Method = method; + context.Request.Path = path; + return (middleware, context, () => wasCalled); + } +} diff --git a/tests/TechHub.Web.Tests/Telemetry/BlazorHubNoiseSuppressorTests.cs b/tests/TechHub.Web.Tests/Telemetry/BlazorHubNoiseSuppressorTests.cs new file mode 100644 index 000000000..cc204c899 --- /dev/null +++ b/tests/TechHub.Web.Tests/Telemetry/BlazorHubNoiseSuppressorTests.cs @@ -0,0 +1,106 @@ +using System.Diagnostics; +using FluentAssertions; +using TechHub.Web.Telemetry; + +namespace TechHub.Web.Tests.Telemetry; + +public class BlazorHubNoiseSuppressorTests +{ + // ─── Failed ComponentHub activities are suppressed ─────────────────────── + + [Fact] + public void OnEnd_UpdateRootComponentsActivity_ErrorStatus_ClearsRecordedFlag() + { + var suppressor = new BlazorHubNoiseSuppressor(); + using var activity = CreateActivity( + "Microsoft.AspNetCore.Components.Server.ComponentHub/UpdateRootComponents", + ActivityStatusCode.Error); + + suppressor.OnEnd(activity); + + activity.ActivityTraceFlags.HasFlag(ActivityTraceFlags.Recorded) + .Should().BeFalse("failed ComponentHub activities must not be exported to App Insights"); + } + + [Theory] + [InlineData("Microsoft.AspNetCore.Components.Server.ComponentHub/UpdateRootComponents")] + [InlineData("Microsoft.AspNetCore.Components.Server.ComponentHub/BeginInvokeDotNetFromJS")] + [InlineData("Microsoft.AspNetCore.Components.Server.ComponentHub/EndInvokeJSFromDotNet")] + [InlineData("Microsoft.AspNetCore.Components.Server.ComponentHub/DispatchBrowserEvent")] + [InlineData("Microsoft.AspNetCore.Components.Server.ComponentHub")] + public void OnEnd_AnyComponentHubActivity_ErrorStatus_ClearsRecordedFlag(string displayName) + { + var suppressor = new BlazorHubNoiseSuppressor(); + using var activity = CreateActivity(displayName, ActivityStatusCode.Error); + + suppressor.OnEnd(activity); + + activity.ActivityTraceFlags.HasFlag(ActivityTraceFlags.Recorded) + .Should().BeFalse($"failed activity '{displayName}' must have Recorded flag cleared"); + } + + // ─── Successful ComponentHub activities pass through untouched ──────────── + + [Theory] + [InlineData(ActivityStatusCode.Ok)] + [InlineData(ActivityStatusCode.Unset)] + public void OnEnd_ComponentHubActivity_NonErrorStatus_LeavesRecordedFlagUnchanged(ActivityStatusCode statusCode) + { + var suppressor = new BlazorHubNoiseSuppressor(); + using var activity = CreateActivity( + "Microsoft.AspNetCore.Components.Server.ComponentHub/UpdateRootComponents", + statusCode); + + suppressor.OnEnd(activity); + + activity.ActivityTraceFlags.HasFlag(ActivityTraceFlags.Recorded) + .Should().BeTrue("successful hub invocations must remain visible in App Insights"); + } + + // ─── Non-ComponentHub activities are left untouched ────────────────────── + + [Theory] + [InlineData("GET /ai/videos/my-article")] + [InlineData("POST /admin/login")] + [InlineData("Microsoft.AspNetCore.Hosting.HttpRequestIn")] + [InlineData("Microsoft.AspNetCore.SignalR.HubInvocation")] + [InlineData("SomeOtherHub/UpdateRootComponents")] + public void OnEnd_NonComponentHubActivity_LeavesRecordedFlagUnchanged(string displayName) + { + var suppressor = new BlazorHubNoiseSuppressor(); + using var activity = CreateActivity(displayName, ActivityStatusCode.Error); // even with Error status + + suppressor.OnEnd(activity); + + activity.ActivityTraceFlags.HasFlag(ActivityTraceFlags.Recorded) + .Should().BeTrue($"activity '{displayName}' must not have its Recorded flag cleared"); + } + + [Fact] + public void OnEnd_ActivityAlreadyNotRecorded_RemainsNotRecorded() + { + // Verifies the bitwise operation does not inadvertently flip flags on unrecorded + // ComponentHub activities that were already suppressed upstream. + var suppressor = new BlazorHubNoiseSuppressor(); + using var activity = new Activity( + "Microsoft.AspNetCore.Components.Server.ComponentHub/UpdateRootComponents"); + + activity.SetStatus(ActivityStatusCode.Error); + activity.ActivityTraceFlags &= ~ActivityTraceFlags.Recorded; // already not recorded + + suppressor.OnEnd(activity); + + activity.ActivityTraceFlags.HasFlag(ActivityTraceFlags.Recorded).Should().BeFalse(); + } + + // ─── Helper ─────────────────────────────────────────────────────────────── + + private static Activity CreateActivity(string displayName, ActivityStatusCode statusCode = ActivityStatusCode.Unset) + { + var activity = new Activity(displayName); + activity.ActivityTraceFlags |= ActivityTraceFlags.Recorded; + activity.Start(); + activity.SetStatus(statusCode); + return activity; + } +}