Refactor event integration service collection extensions into their own extension

[brant-livefront]

📔 Objective

As the event integration services grew, they were simply being added to the large ServiceCollectionExtensions in SharedWeb. This PR refactors them into their own self-contained extension. This gives us the advantage of not having so much done in one file, but also added an easy place to add extensive tests for all of these (where there previously had been no tests).

This is primarily simply a migration. However there are 2 functional changes:

1. We were previously calling AddDistributedCache inside of the event integration functions. This is not possible when moving outside of SharedWeb. Instead, we are now calling AddDistributedCache in the Startup.cs in Events and EventsProcessor (which follows the pattern in other apps - Api, Billing, etc). This has been documented as a requirement in the XMLDoc for these methods
2. In writing tests I realized that we supported launching listeners for both Azure Service Bus and RabbitMQ if you had both of them configured. This is likely a misconfiguration and is out of step with how we handle precedence on the write side. I've updated the logic to explicitly prefer ASB and register only ASB if they are both configured (as well as adding a test to exercise this scenario explicitly).

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[claude[bot]]

Claude finished @brant-livefront's task in 1m 34s —— View job

---

Code Review Complete ✓

I've reviewed this refactoring PR and it looks solid overall. The changes successfully extract event integration service registration into a dedicated, well-tested extension file. Here are my findings:

Strengths 👍

1. Excellent Test Coverage: The new test file has 667 lines of comprehensive tests covering:

   • Service registration and idempotency
   • Configuration precedence (Azure Service Bus over RabbitMQ)
   • All configuration permutations
   • Service lifetimes and scoping

2. Clear Documentation: XML docs thoroughly explain each method's purpose, prerequisites, and behavior

3. Improved Organization: Moving ~300 lines from SharedWeb into a focused extension improves maintainability

4. Proper Dependency Injection Patterns: Uses TryAdd methods following ADR-0026

5. Sensible Precedence Logic: Azure Service Bus over RabbitMQ is well-documented and tested (lines 644-669 in tests)

Areas of Concern ⚠️

1. Distributed Cache Prerequisite (EventIntegrationsServiceCollectionExtensions.cs:130, 171, 274)

   • The XML docs state callers "must ensure AddDistributedCache has been called"
   • However, there's no runtime validation to enforce this requirement
   • If a caller forgets, they'll get a cryptic DI resolution error at runtime
   • Suggestion: Consider adding a guard check or making the dependency explicit

2. Bot Builder Dependencies in Core.csproj (line 43-45)

   • Microsoft.Bot.Builder packages are now required in Core, which increases the footprint
   • These are only needed when Teams integration is configured
   • The PR reviewer @jrmccannon already confirmed this is for Teams integration
   • Note: This is acceptable but worth documenting as a tradeoff

3. Test Coverage Gap

   • While tests verify service registration, there are no integration tests verifying the startup flows in Events/EventsProcessor actually work correctly with the new prerequisite
   • The 77% patch coverage reported by Codecov suggests some code paths aren't tested

Security & Architecture ✓

• No security concerns identified
• Follows zero-knowledge principles (no vault data handling)
• Consistent with Bitwarden architectural patterns
• No breaking changes beyond documented behavior fixes

Minor Observations 📝

• The functional change to prevent dual ASB+RabbitMQ registration is correct and well-tested
• The Startup.cs changes correctly add AddDistributedCache calls before listener registration
• The precedence logic matches between write services and listener services

Recommendation

This is a well-executed refactoring with excellent test coverage. The code quality is high and follows project conventions. My only suggestion would be to add runtime validation for the distributed cache prerequisite to prevent configuration errors.

Approval: ✅ Ready to merge once CI passes

[sonarqubecloud[bot]]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
2.7% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions[bot]]

Checkmarx One – Scan Summary & Details – 08622646-d1a0-4500-85f8-9523cc7015d9

New Issues (2)

Checkmarx found the following issues in this Pull Request

Severity	Issue	Source File / Package	Checkmarx Insight
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1519	details Method at line 1519 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ... ID: dMGF5qNfAN72zlvQcA1MgbhHv%2Fc%3D Attack Vector
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1395	details Method at line 1395 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ... ID: iOCFr11iI9znjDnv46yLfiS4aDY%3D Attack Vector

Fixed Issues (1)

Great job! The following issues were fixed in this Pull Request

Severity	Issue	Source File / Package
	~~CSRF~~	/src/Api/Vault/Controllers/CiphersController.cs: 300

[codecov[bot]]

Codecov Report

:x: Patch coverage is 77.25322% with 53 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 54.10%. Comparing base (72c8967) to head (2fe2ceb). :warning: Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
...ns/EventIntegrationsServiceCollectionExtensions.cs	77.48%	52 Missing :warning:
src/EventsProcessor/Startup.cs	0.00%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6714      +/-   ##
==========================================
+ Coverage   53.90%   54.10%   +0.19%     
==========================================
  Files        1922     1922              
  Lines       85214    85215       +1     
  Branches     7621     7621              
==========================================
+ Hits        45937    46106     +169     
+ Misses      37511    37352     -159     
+ Partials     1766     1757       -9     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[brant-livefront]

@jrmccannon @withinfocus This is ready for a re-approval after being re-targeted to main (should be no new changes)