[PM-18555] Main part of notifications refactor (#5757)

* More tests * More tests * Add non-guid tests * Introduce slimmer services * Implement IPushEngine on services * Implement IPushEngine * Fix tests * Format * Switch to `Guid` on `PushSendRequestModel` * Remove TODOs
2025-06-30 07:36:14 -05:00 · 2025-06-17 13:30:56 -04:00
parent 6dc26f4be6
commit 6800bc57f3
23 changed files with 1271 additions and 2408 deletions
--- a/test/Core.Test/Platform/Push/Services/AzureQueuePushNotificationServiceTests.cs
+++ b/test/Core.Test/Platform/Push/Services/AzureQueuePushNotificationServiceTests.cs
@ -9,14 +9,11 @@ using Bit.Core.Enums;
 using Bit.Core.Models;
 using Bit.Core.NotificationCenter.Entities;
 using Bit.Core.NotificationCenter.Enums;
+using Bit.Core.Platform.Push;
 using Bit.Core.Platform.Push.Internal;
-using Bit.Core.Settings;
 using Bit.Core.Test.AutoFixture;
-using Bit.Core.Test.AutoFixture.CurrentContextFixtures;
-using Bit.Core.Test.NotificationCenter.AutoFixture;
 using Bit.Core.Tools.Entities;
 using Bit.Core.Vault.Entities;
-using Bit.Test.Common.AutoFixture;
 using Bit.Test.Common.AutoFixture.Attributes;
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.DependencyInjection;
@ -42,96 +39,6 @@ public class AzureQueuePushNotificationServiceTests
        _fakeTimeProvider.SetUtcNow(DateTime.UtcNow);
    }

-    [Theory]
-    [BitAutoData]
-    [NotificationCustomize]
-    [CurrentContextCustomize]
-    public async Task PushNotificationAsync_NotificationGlobal_Sent(
-        SutProvider<AzureQueuePushNotificationService> sutProvider, Notification notification, Guid deviceIdentifier,
-        ICurrentContext currentContext, Guid installationId)
-    {
-        currentContext.DeviceIdentifier.Returns(deviceIdentifier.ToString());
-        sutProvider.GetDependency<IHttpContextAccessor>().HttpContext!.RequestServices
-            .GetService(Arg.Any<Type>()).Returns(currentContext);
-        sutProvider.GetDependency<IGlobalSettings>().Installation.Id = installationId;
-
-        await sutProvider.Sut.PushNotificationAsync(notification);
-
-        await sutProvider.GetDependency<QueueClient>().Received(1)
-            .SendMessageAsync(Arg.Is<string>(message =>
-                MatchMessage(PushType.Notification, message,
-                    new NotificationPushNotificationEquals(notification, null, installationId),
-                    deviceIdentifier.ToString())));
-    }
-
-    [Theory]
-    [BitAutoData]
-    [NotificationCustomize(false)]
-    [CurrentContextCustomize]
-    public async Task PushNotificationAsync_NotificationNotGlobal_Sent(
-        SutProvider<AzureQueuePushNotificationService> sutProvider, Notification notification, Guid deviceIdentifier,
-        ICurrentContext currentContext, Guid installationId)
-    {
-        currentContext.DeviceIdentifier.Returns(deviceIdentifier.ToString());
-        sutProvider.GetDependency<IHttpContextAccessor>().HttpContext!.RequestServices
-            .GetService(Arg.Any<Type>()).Returns(currentContext);
-        sutProvider.GetDependency<IGlobalSettings>().Installation.Id = installationId;
-
-        await sutProvider.Sut.PushNotificationAsync(notification);
-
-        await sutProvider.GetDependency<QueueClient>().Received(1)
-            .SendMessageAsync(Arg.Is<string>(message =>
-                MatchMessage(PushType.Notification, message,
-                    new NotificationPushNotificationEquals(notification, null, null),
-                    deviceIdentifier.ToString())));
-    }
-
-    [Theory]
-    [BitAutoData]
-    [NotificationCustomize]
-    [NotificationStatusCustomize]
-    [CurrentContextCustomize]
-    public async Task PushNotificationStatusAsync_NotificationGlobal_Sent(
-        SutProvider<AzureQueuePushNotificationService> sutProvider, Notification notification, Guid deviceIdentifier,
-        ICurrentContext currentContext, NotificationStatus notificationStatus, Guid installationId)
-    {
-        currentContext.DeviceIdentifier.Returns(deviceIdentifier.ToString());
-        sutProvider.GetDependency<IHttpContextAccessor>().HttpContext!.RequestServices
-            .GetService(Arg.Any<Type>()).Returns(currentContext);
-        sutProvider.GetDependency<IGlobalSettings>().Installation.Id = installationId;
-
-        await sutProvider.Sut.PushNotificationStatusAsync(notification, notificationStatus);
-
-        await sutProvider.GetDependency<QueueClient>().Received(1)
-            .SendMessageAsync(Arg.Is<string>(message =>
-                MatchMessage(PushType.NotificationStatus, message,
-                    new NotificationPushNotificationEquals(notification, notificationStatus, installationId),
-                    deviceIdentifier.ToString())));
-    }
-
-    [Theory]
-    [BitAutoData]
-    [NotificationCustomize(false)]
-    [NotificationStatusCustomize]
-    [CurrentContextCustomize]
-    public async Task PushNotificationStatusAsync_NotificationNotGlobal_Sent(
-        SutProvider<AzureQueuePushNotificationService> sutProvider, Notification notification, Guid deviceIdentifier,
-        ICurrentContext currentContext, NotificationStatus notificationStatus, Guid installationId)
-    {
-        currentContext.DeviceIdentifier.Returns(deviceIdentifier.ToString());
-        sutProvider.GetDependency<IHttpContextAccessor>().HttpContext!.RequestServices
-            .GetService(Arg.Any<Type>()).Returns(currentContext);
-        sutProvider.GetDependency<IGlobalSettings>().Installation.Id = installationId;
-
-        await sutProvider.Sut.PushNotificationStatusAsync(notification, notificationStatus);
-
-        await sutProvider.GetDependency<QueueClient>().Received(1)
-            .SendMessageAsync(Arg.Is<string>(message =>
-                MatchMessage(PushType.NotificationStatus, message,
-                    new NotificationPushNotificationEquals(notification, notificationStatus, null),
-                    deviceIdentifier.ToString())));
-    }
-
    [Theory]
    [InlineData("6a5bbe1b-cf16-49a6-965f-5c2eac56a531", null)]
    [InlineData(null, "b9a3fcb4-2447-45c1-aad2-24de43c88c44")]
@ -844,7 +751,7 @@ public class AzureQueuePushNotificationServiceTests
    //     );
    // }

-    private async Task VerifyNotificationAsync(Func<AzureQueuePushNotificationService, Task> test, JsonNode expectedMessage)
+    private async Task VerifyNotificationAsync(Func<IPushNotificationService, Task> test, JsonNode expectedMessage)
    {
        var queueClient = Substitute.For<QueueClient>();

@ -872,7 +779,7 @@ public class AzureQueuePushNotificationServiceTests
            _fakeTimeProvider
        );

-        await test(sut);
+        await test(new EngineWrapper(sut, _fakeTimeProvider, _globalSettings.Installation.Id));

        // Hoist equality checker outside the expression so that we
        // can more easily place a breakpoint
--- a/test/Core.Test/Platform/Push/Services/MultiServicePushNotificationServiceTests.cs
+++ b/test/Core.Test/Platform/Push/Services/MultiServicePushNotificationServiceTests.cs
@ -1,98 +1,8 @@
 #nullable enable
-using Bit.Core.Enums;
-using Bit.Core.NotificationCenter.Entities;
-using Bit.Core.Platform.Push;
-using Bit.Core.Platform.Push.Internal;
-using Bit.Core.Test.NotificationCenter.AutoFixture;
-using Bit.Test.Common.AutoFixture;
-using Bit.Test.Common.AutoFixture.Attributes;
-using NSubstitute;
-using Xunit;

 namespace Bit.Core.Test.Platform.Push.Services;

-[SutProviderCustomize]
 public class MultiServicePushNotificationServiceTests
 {
-    [Theory]
-    [BitAutoData]
-    [NotificationCustomize]
-    public async Task PushNotificationAsync_Notification_Sent(
-        SutProvider<MultiServicePushNotificationService> sutProvider, Notification notification)
-    {
-        await sutProvider.Sut.PushNotificationAsync(notification);
-
-        await sutProvider.GetDependency<IEnumerable<IPushNotificationService>>()
-            .First()
-            .Received(1)
-            .PushNotificationAsync(notification);
-    }
-
-    [Theory]
-    [BitAutoData]
-    [NotificationCustomize]
-    [NotificationStatusCustomize]
-    public async Task PushNotificationStatusAsync_Notification_Sent(
-        SutProvider<MultiServicePushNotificationService> sutProvider, Notification notification,
-        NotificationStatus notificationStatus)
-    {
-        await sutProvider.Sut.PushNotificationStatusAsync(notification, notificationStatus);
-
-        await sutProvider.GetDependency<IEnumerable<IPushNotificationService>>()
-            .First()
-            .Received(1)
-            .PushNotificationStatusAsync(notification, notificationStatus);
-    }
-
-    [Theory]
-    [BitAutoData([null, null])]
-    [BitAutoData(ClientType.All, null)]
-    [BitAutoData([null, "test device id"])]
-    [BitAutoData(ClientType.All, "test device id")]
-    public async Task SendPayloadToUserAsync_Message_Sent(ClientType? clientType, string? deviceId, string userId,
-        PushType type, object payload, string identifier, SutProvider<MultiServicePushNotificationService> sutProvider)
-    {
-        await sutProvider.Sut.SendPayloadToUserAsync(userId, type, payload, identifier, deviceId, clientType);
-
-        await sutProvider.GetDependency<IEnumerable<IPushNotificationService>>()
-            .First()
-            .Received(1)
-            .SendPayloadToUserAsync(userId, type, payload, identifier, deviceId, clientType);
-    }
-
-    [Theory]
-    [BitAutoData([null, null])]
-    [BitAutoData(ClientType.All, null)]
-    [BitAutoData([null, "test device id"])]
-    [BitAutoData(ClientType.All, "test device id")]
-    public async Task SendPayloadToOrganizationAsync_Message_Sent(ClientType? clientType, string? deviceId,
-        string organizationId, PushType type, object payload, string identifier,
-        SutProvider<MultiServicePushNotificationService> sutProvider)
-    {
-        await sutProvider.Sut.SendPayloadToOrganizationAsync(organizationId, type, payload, identifier, deviceId,
-            clientType);
-
-        await sutProvider.GetDependency<IEnumerable<IPushNotificationService>>()
-            .First()
-            .Received(1)
-            .SendPayloadToOrganizationAsync(organizationId, type, payload, identifier, deviceId, clientType);
-    }
-
-    [Theory]
-    [BitAutoData([null, null])]
-    [BitAutoData(ClientType.All, null)]
-    [BitAutoData([null, "test device id"])]
-    [BitAutoData(ClientType.All, "test device id")]
-    public async Task SendPayloadToInstallationAsync_Message_Sent(ClientType? clientType, string? deviceId,
-        string installationId, PushType type, object payload, string identifier,
-        SutProvider<MultiServicePushNotificationService> sutProvider)
-    {
-        await sutProvider.Sut.SendPayloadToInstallationAsync(installationId, type, payload, identifier, deviceId,
-            clientType);
-
-        await sutProvider.GetDependency<IEnumerable<IPushNotificationService>>()
-            .First()
-            .Received(1)
-            .SendPayloadToInstallationAsync(installationId, type, payload, identifier, deviceId, clientType);
-    }
+    // TODO: Can add a couple tests here
 }
--- a/test/Core.Test/Platform/Push/Services/NotificationsApiPushNotificationServiceTests.cs
+++ b/test/Core.Test/Platform/Push/Services/NotificationsApiPushNotificationServiceTests.cs
@ -19,14 +19,13 @@ public class NotificationsApiPushNotificationServiceTests : PushTestBase

    protected override string ExpectedClientUrl() => "https://localhost:7777/send";

-    protected override IPushNotificationService CreateService()
+    protected override IPushEngine CreateService()
    {
        return new NotificationsApiPushNotificationService(
            HttpClientFactory,
            GlobalSettings,
            HttpContextAccessor,
-            NullLogger<NotificationsApiPushNotificationService>.Instance,
-            FakeTimeProvider
+            NullLogger<NotificationsApiPushNotificationService>.Instance
        );
    }

@ -221,7 +220,7 @@ public class NotificationsApiPushNotificationServiceTests : PushTestBase
                ["UserId"] = send.UserId,
                ["RevisionDate"] = send.RevisionDate,
            },
-            ["ContextId"] = null,
+            ["ContextId"] = DeviceIdentifier,
        };
    }

@ -236,7 +235,7 @@ public class NotificationsApiPushNotificationServiceTests : PushTestBase
                ["UserId"] = send.UserId,
                ["RevisionDate"] = send.RevisionDate,
            },
-            ["ContextId"] = null,
+            ["ContextId"] = DeviceIdentifier,
        };
    }

@ -251,7 +250,7 @@ public class NotificationsApiPushNotificationServiceTests : PushTestBase
                ["UserId"] = send.UserId,
                ["RevisionDate"] = send.RevisionDate,
            },
-            ["ContextId"] = null,
+            ["ContextId"] = DeviceIdentifier,
        };
    }

--- a/test/Core.Test/Platform/Push/Services/PushTestBase.cs
+++ b/test/Core.Test/Platform/Push/Services/PushTestBase.cs
@ -15,11 +15,28 @@ using Bit.Core.Tools.Entities;
 using Bit.Core.Vault.Entities;
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Abstractions;
 using Microsoft.Extensions.Time.Testing;
 using NSubstitute;
 using RichardSzalay.MockHttp;
 using Xunit;

+public class EngineWrapper(IPushEngine pushEngine, FakeTimeProvider fakeTimeProvider, Guid installationId) : IPushNotificationService
+{
+    public Guid InstallationId { get; } = installationId;
+
+    public TimeProvider TimeProvider { get; } = fakeTimeProvider;
+
+    public ILogger Logger => NullLogger<EngineWrapper>.Instance;
+
+    public Task PushAsync<T>(PushNotification<T> pushNotification) where T : class
+        => pushEngine.PushAsync(pushNotification);
+
+    public Task PushCipherAsync(Cipher cipher, PushType pushType, IEnumerable<Guid>? collectionIds)
+        => pushEngine.PushCipherAsync(cipher, pushType, collectionIds);
+}
+
 public abstract class PushTestBase
 {
    protected static readonly string DeviceIdentifier = "test_device_identifier";
@ -51,7 +68,7 @@ public abstract class PushTestBase
        FakeTimeProvider.SetUtcNow(DateTimeOffset.UtcNow);
    }

-    protected abstract IPushNotificationService CreateService();
+    protected abstract IPushEngine CreateService();

    protected abstract string ExpectedClientUrl();

@ -480,7 +497,7 @@ public abstract class PushTestBase
            })
            .Respond(HttpStatusCode.OK);

-        await test(CreateService());
+        await test(new EngineWrapper(CreateService(), FakeTimeProvider, GlobalSettings.Installation.Id));

        Assert.NotNull(actualNode);

--- a/test/Core.Test/Platform/Push/Services/RelayPushNotificationServiceTests.cs
+++ b/test/Core.Test/Platform/Push/Services/RelayPushNotificationServiceTests.cs
@ -4,8 +4,8 @@ using System.Text.Json.Nodes;
 using Bit.Core.AdminConsole.Entities;
 using Bit.Core.Auth.Entities;
 using Bit.Core.Entities;
-using Bit.Core.Enums;
 using Bit.Core.NotificationCenter.Entities;
+using Bit.Core.Platform.Push;
 using Bit.Core.Platform.Push.Internal;
 using Bit.Core.Repositories;
 using Bit.Core.Settings;
@ -14,7 +14,6 @@ using Bit.Core.Vault.Entities;
 using Microsoft.Extensions.Logging.Abstractions;
 using Microsoft.Extensions.Time.Testing;
 using NSubstitute;
-using Xunit;

 namespace Bit.Core.Test.Platform.Push.Services;

@ -38,47 +37,19 @@ public class RelayPushNotificationServiceTests : PushTestBase
        GlobalSettings.Installation.IdentityUri = "https://localhost:8888";
    }

-    protected override RelayPushNotificationService CreateService()
+    protected override IPushEngine CreateService()
    {
        return new RelayPushNotificationService(
            HttpClientFactory,
            _deviceRepository,
            GlobalSettings,
            HttpContextAccessor,
-            NullLogger<RelayPushNotificationService>.Instance,
-            FakeTimeProvider
+            NullLogger<RelayPushNotificationService>.Instance
        );
    }

    protected override string ExpectedClientUrl() => "https://localhost:7777/push/send";

-    [Fact]
-    public async Task SendPayloadToInstallationAsync_ThrowsNotImplementedException()
-    {
-        var sut = CreateService();
-        await Assert.ThrowsAsync<NotImplementedException>(
-            async () => await sut.SendPayloadToInstallationAsync("installation_id", PushType.AuthRequest, new { }, null)
-        );
-    }
-
-    [Fact]
-    public async Task SendPayloadToUserAsync_ThrowsNotImplementedException()
-    {
-        var sut = CreateService();
-        await Assert.ThrowsAsync<NotImplementedException>(
-            async () => await sut.SendPayloadToUserAsync("user_id", PushType.AuthRequest, new { }, null)
-        );
-    }
-
-    [Fact]
-    public async Task SendPayloadToOrganizationAsync_ThrowsNotImplementedException()
-    {
-        var sut = CreateService();
-        await Assert.ThrowsAsync<NotImplementedException>(
-            async () => await sut.SendPayloadToOrganizationAsync("organization_id", PushType.AuthRequest, new { }, null)
-        );
-    }
-
    protected override JsonNode GetPushSyncCipherCreatePayload(Cipher cipher, Guid collectionIds)
    {
        return new JsonObject