Prepare IPushNotificationService for extraction into a push library - #8199
Draft
justindbaur wants to merge 6 commits into
Draft
Prepare IPushNotificationService for extraction into a push library#8199justindbaur wants to merge 6 commits into
justindbaur wants to merge 6 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.41% 63.18% -5.23%
==========================================
Files 2380 2397 +17
Lines 103679 103755 +76
Branches 9386 9385 -1
==========================================
- Hits 70931 65557 -5374
- Misses 30420 35956 +5536
+ Partials 2328 2242 -86 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎟️ 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