Prepare IPushNotificationService for extraction into a push library - #8199
Prepare IPushNotificationService for extraction into a push library#8199justindbaur wants to merge 7 commits into
Conversation
- Remove all 16 default interface methods (DIMs) from IPushNotificationService, leaving only PushAsync<T>; the three legacy properties (InstallationId, TimeProvider, Logger) are retained but marked [Obsolete(DiagnosticId="BWP0001")] - Extract each DIM as a static extension method in its owning team's domain folder (Auth, Vault, Tools, AdminConsole, NotificationCenter, Billing) - Move push notification payload types out of PushNotification.cs into their owning team's Models folder; namespace stays Bit.Core.Models to avoid touching callers (file location is the ownership signal, not the namespace—see comment in each file) - Fix LogOut PushType attribution from "not-specified" to @bitwarden/team-auth-dev - Update all service-level tests to assert on PushAsync<T> instead of the removed DIMs; delete the snapshot-style PushTestBase engine tests that were migration guards rather than living contracts - Add AzureQueuePushEngineWireFormatTests as a wire-format guard and HubHelpers routing guard for the Azure Queue push path
Replaces the mock-based AzureQueuePushEngineWireFormatTests with a true end-to-end pipeline test (AzureQueuePipelineTests) that drives the full chain: AzureQueuePushEngine → ChannelQueueClient → AzureQueueHostedService → HubHelpers → SignalR hub mocks. - ChannelQueueClient: in-memory QueueClient backed by System.Threading.Channels, with a separate capture channel so producer and consumer sides don't race - AzureQueuePipelineTests: wire-format guard + routing guard + ClientType routing, including the AuthRequestResponse → anonymous hub case - NotificationsApplicationFactory: expose AnonymousHubClients for asserting auth-request-response routing - PostSendEndpointTests: add AuthRequestResponse routing case (anonymous hub) - AzureQueueHostedService: inject TimeProvider for delay control in tests; resolve QueueClient via keyed service provider - Startup: register TimeProvider.System and QueueClient as keyed singleton so tests can substitute ChannelQueueClient
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8199 +/- ##
==========================================
- Coverage 68.56% 63.22% -5.35%
==========================================
Files 2381 2397 +16
Lines 103902 103900 -2
Branches 9405 9405
==========================================
- Hits 71245 65695 -5550
- Misses 30310 35947 +5637
+ Partials 2347 2258 -89 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the removal of all 18 default interface methods from Code Review Details
Also noted, not blocking: the extracted |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
harr1424
left a comment
harr1424
left a comment
There was a problem hiding this comment.
Changes to Tools-owned files look good ✅
…s-tests # Conflicts: # test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs # test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs
4 participants
🎟️ Tracking
Part of: https://bitwarden.atlassian.net/browse/PM-34546
📔 Objective
Removes all 16 default interface methods (DIMs) from
IPushNotificationService, leaving onlyPushAsync<T>. Each DIM is extracted as a static extension method in its owning team's domain folder (Auth, Vault, Tools, AdminConsole, NotificationCenter, Billing). Push notification payload types are moved into their owning team'sModelsfolder (namespace kept asBit.Core.Modelsto avoid touching callers).Also adds end-to-end pipeline tests for the Azure Queue push path:
AzureQueuePipelineTests— drives the full chain (AzureQueuePushEngine→ in-memoryChannelQueueClient→AzureQueueHostedService→HubHelpers→ SignalR hub mocks), covering wire-format contracts, routing by user/org/installation/client-type, and theAuthRequestResponse→ anonymous hub pathPostSendEndpointTests— integration tests forPOST /sendusing a real in-memory Identity server and JWT tokens, including the anonymous hub routing caseAzureQueueHostedServicegetsTimeProviderinjection and resolvesQueueClientvia keyed DI so tests can substituteChannelQueueClientwithout touching production wiringKey changes:
IPushNotificationService; retain the three legacy properties marked[Obsolete(DiagnosticId="BWP0001")]Modelsfolders (namespace unchanged)PushTestBaseengine tests (migration guards, not living contracts)AzureQueuePipelineTests+ChannelQueueClientas the replacement living contractOnce these changes land, Push should be able to be made into a library and Notifications should be able to remove a dependency on
Corewith only Platform review for future PRs.📸 Screenshots
N/A