PM-10600: Small refactor, UTs coverage

src/Core/NotificationHub/INotificationHubPool.cs

@@ -4,6 +4,6 @@ namespace Bit.Core.NotificationHub;
 
 public interface INotificationHubPool
 {
-    NotificationHubClient ClientFor(Guid comb);
+    INotificationHubClient ClientFor(Guid comb);
     INotificationHubProxy AllClients { get; }
 }

src/Core/NotificationHub/NotificationHubPool.cs

@@ -43,7 +43,7 @@ public class NotificationHubPool : INotificationHubPool
     /// <param name="comb"></param>
     /// <returns></returns>
     /// <exception cref="InvalidOperationException">Thrown when no notification hub is found for a given comb.</exception>
-    public NotificationHubClient ClientFor(Guid comb)
+    public INotificationHubClient ClientFor(Guid comb)
     {
         var possibleConnections = _connections.Where(c => c.RegistrationEnabled(comb)).ToArray();
         if (possibleConnections.Length == 0)

src/Core/NotificationHub/NotificationHubPushRegistrationService.cs

@@ -2,34 +2,25 @@
 using Bit.Core.Models.Data;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
-using Bit.Core.Settings;
 using Bit.Core.Utilities;
 using Microsoft.Azure.NotificationHubs;
-using Microsoft.Extensions.DependencyInjection;
-using Microsoft.Extensions.Logging;
 
 namespace Bit.Core.NotificationHub;
 
 public class NotificationHubPushRegistrationService : IPushRegistrationService
 {
     private readonly IInstallationDeviceRepository _installationDeviceRepository;
-    private readonly GlobalSettings _globalSettings;
     private readonly INotificationHubPool _notificationHubPool;
-    private readonly IServiceProvider _serviceProvider;
+    private readonly IFeatureService _featureService;
-    private readonly ILogger<NotificationHubPushRegistrationService> _logger;
 
     public NotificationHubPushRegistrationService(
         IInstallationDeviceRepository installationDeviceRepository,
-        GlobalSettings globalSettings,
         INotificationHubPool notificationHubPool,
-        IServiceProvider serviceProvider,
+        IFeatureService featureService)
-        ILogger<NotificationHubPushRegistrationService> logger)
     {
         _installationDeviceRepository = installationDeviceRepository;
-        _globalSettings = globalSettings;
         _notificationHubPool = notificationHubPool;
-        _serviceProvider = serviceProvider;
+        _featureService = featureService;
-        _logger = logger;
     }
 
     public async Task CreateOrUpdateRegistrationAsync(string pushToken, string deviceId, string userId,
@@ -60,8 +51,7 @@ public class NotificationHubPushRegistrationService : IPushRegistrationService
         switch (type)
         {
             case DeviceType.Android:
-                var featureService = _serviceProvider.GetRequiredService<IFeatureService>();
+                if (_featureService.IsEnabled(FeatureFlagKeys.AnhFcmv1Migration))
-                if (featureService.IsEnabled(FeatureFlagKeys.AnhFcmv1Migration))
                 {
                     payloadTemplate = "{\"message\":{\"data\":{\"type\":\"$(type)\",\"payload\":\"$(payload)\"}}}";
                     messageTemplate = "{\"message\":{\"data\":{\"type\":\"$(type)\"}," +
@@ -196,7 +186,8 @@ public class NotificationHubPushRegistrationService : IPushRegistrationService
         {
             try
             {
-                await ClientFor(GetComb(deviceId)).PatchInstallationAsync(deviceId, new List<PartialUpdateOperation> { operation });
+                await ClientFor(GetComb(deviceId))
+                    .PatchInstallationAsync(deviceId, new List<PartialUpdateOperation> { operation });
             }
             catch (Exception e) when (e.InnerException == null || !e.InnerException.Message.Contains("(404) Not Found"))
             {
@@ -205,29 +196,24 @@ public class NotificationHubPushRegistrationService : IPushRegistrationService
         }
     }
 
-    private NotificationHubClient ClientFor(Guid deviceId)
+    private INotificationHubClient ClientFor(Guid deviceId)
     {
         return _notificationHubPool.ClientFor(deviceId);
     }
 
     private Guid GetComb(string deviceId)
     {
-        var deviceIdString = deviceId;
+        if (InstallationDeviceEntity.TryParse(deviceId, out var installationDeviceEntity))
-        InstallationDeviceEntity installationDeviceEntity;
-        Guid deviceIdGuid;
-        if (InstallationDeviceEntity.TryParse(deviceIdString, out installationDeviceEntity))
         {
             // Strip off the installation id (PartitionId). RowKey is the ID in the Installation's table.
-            deviceIdString = installationDeviceEntity.RowKey;
+            deviceId = installationDeviceEntity.RowKey;
         }
 
-        if (Guid.TryParse(deviceIdString, out deviceIdGuid))
+        if (!Guid.TryParse(deviceId, out var deviceIdGuid))
-        {
-        }
-        else
         {
             throw new Exception($"Invalid device id {deviceId}.");
         }
+
         return deviceIdGuid;
     }
 }

test/Core.Test/NotificationHub/NotificationHubPushRegistrationServiceTests.cs

@@ -1,44 +1,269 @@
-using Bit.Core.NotificationHub;
+#nullable enable
-using Bit.Core.Repositories;
+using Bit.Core.Enums;
-using Bit.Core.Settings;
+using Bit.Core.NotificationHub;
-using Microsoft.Extensions.Logging;
+using Bit.Core.Services;
+using Bit.Core.Utilities;
+using Bit.Test.Common.AutoFixture;
+using Bit.Test.Common.AutoFixture.Attributes;
+using Microsoft.Azure.NotificationHubs;
 using NSubstitute;
 using Xunit;
 
 namespace Bit.Core.Test.NotificationHub;
 
+[SutProviderCustomize]
 public class NotificationHubPushRegistrationServiceTests
 {
-    private readonly NotificationHubPushRegistrationService _sut;
+    [Theory]
-
+    [BitAutoData([null])]
-    private readonly IInstallationDeviceRepository _installationDeviceRepository;
+    [BitAutoData("")]
-    private readonly IServiceProvider _serviceProvider;
+    [BitAutoData(" ")]
-    private readonly ILogger<NotificationHubPushRegistrationService> _logger;
+    public async void CreateOrUpdateRegistrationAsync_PushTokenNullOrEmpty_InstallationNotCreated(string? pushToken,
-    private readonly GlobalSettings _globalSettings;
+        SutProvider<NotificationHubPushRegistrationService> sutProvider, Guid deviceId, Guid userId, Guid identifier)
-    private readonly INotificationHubPool _notificationHubPool;
-
-    public NotificationHubPushRegistrationServiceTests()
     {
-        _installationDeviceRepository = Substitute.For<IInstallationDeviceRepository>();
+        await sutProvider.Sut.CreateOrUpdateRegistrationAsync(pushToken, deviceId.ToString(), userId.ToString(),
-        _serviceProvider = Substitute.For<IServiceProvider>();
+            identifier.ToString(), DeviceType.Android);
-        _logger = Substitute.For<ILogger<NotificationHubPushRegistrationService>>();
-        _globalSettings = new GlobalSettings();
-        _notificationHubPool = Substitute.For<INotificationHubPool>();
 
-        _sut = new NotificationHubPushRegistrationService(
+        sutProvider.GetDependency<INotificationHubPool>()
-            _installationDeviceRepository,
+            .Received(0)
-            _globalSettings,
+            .ClientFor(deviceId);
-            _notificationHubPool,
-            _serviceProvider,
-            _logger
-        );
     }
 
-    // Remove this test when we add actual tests. It only proves that
+    [Theory]
-    // we've properly constructed the system under test.
+    [BitAutoData(false)]
-    [Fact(Skip = "Needs additional work")]
+    [BitAutoData(true)]
-    public void ServiceExists()
+    public async void CreateOrUpdateRegistrationAsync_DeviceTypeAndroid_InstallationCreated(bool identifierNull,
+        SutProvider<NotificationHubPushRegistrationService> sutProvider, Guid deviceId, Guid userId, Guid? identifier)
     {
-        Assert.NotNull(_sut);
+        sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.AnhFcmv1Migration).Returns(true);
+        var notificationHubClient = Substitute.For<INotificationHubClient>();
+        sutProvider.GetDependency<INotificationHubPool>().ClientFor(Arg.Any<Guid>()).Returns(notificationHubClient);
+
+        var pushToken = "test push token";
+
+        await sutProvider.Sut.CreateOrUpdateRegistrationAsync(pushToken, deviceId.ToString(), userId.ToString(),
+            identifierNull ? null : identifier.ToString(), DeviceType.Android);
+
+        sutProvider.GetDependency<INotificationHubPool>()
+            .Received(1)
+            .ClientFor(deviceId);
+        await notificationHubClient
+            .Received(1)
+            .CreateOrUpdateInstallationAsync(Arg.Is<Installation>(installation =>
+                installation.InstallationId == deviceId.ToString() &&
+                installation.PushChannel == pushToken &&
+                installation.Platform == NotificationPlatform.FcmV1 &&
+                installation.Tags.Count == (identifierNull ? 2 : 3) &&
+                installation.Tags.Contains($"userId:{userId}") &&
+                installation.Tags.Contains("clientType:Mobile") &&
+                (identifierNull || installation.Tags.Contains($"deviceIdentifier:{identifier}")) &&
+                installation.Templates.Count == 3));
+        await notificationHubClient
+            .Received(1)
+            .CreateOrUpdateInstallationAsync(Arg.Is<Installation>(installation => MatchingInstallationTemplate(
+                installation.Templates, "template:payload",
+                "{\"message\":{\"data\":{\"type\":\"$(type)\",\"payload\":\"$(payload)\"}}}",
+                new List<string?>
+                {
+                    "template:payload",
+                    $"template:payload_userId:{userId}",
+                    "clientType:Mobile",
+                    identifierNull ? null : $"template:payload_deviceIdentifier:{identifier}"
+                })));
+        await notificationHubClient
+            .Received(1)
+            .CreateOrUpdateInstallationAsync(Arg.Is<Installation>(installation => MatchingInstallationTemplate(
+                installation.Templates, "template:message",
+                "{\"message\":{\"data\":{\"type\":\"$(type)\"},\"notification\":{\"title\":\"$(title)\",\"body\":\"$(message)\"}}}",
+                new List<string?>
+                {
+                    "template:message",
+                    $"template:message_userId:{userId}",
+                    "clientType:Mobile",
+                    identifierNull ? null : $"template:message_deviceIdentifier:{identifier}"
+                })));
+        await notificationHubClient
+            .Received(1)
+            .CreateOrUpdateInstallationAsync(Arg.Is<Installation>(installation => MatchingInstallationTemplate(
+                installation.Templates, "template:badgeMessage",
+                "{\"message\":{\"data\":{\"type\":\"$(type)\"},\"notification\":{\"title\":\"$(title)\",\"body\":\"$(message)\"}}}",
+                new List<string?>
+                {
+                    "template:badgeMessage",
+                    $"template:badgeMessage_userId:{userId}",
+                    "clientType:Mobile",
+                    identifierNull ? null : $"template:badgeMessage_deviceIdentifier:{identifier}"
+                })));
+    }
+
+    [Theory]
+    [BitAutoData(false)]
+    [BitAutoData(true)]
+    public async void CreateOrUpdateRegistrationAsync_DeviceTypeIOS_InstallationCreated(bool identifierNull,
+        SutProvider<NotificationHubPushRegistrationService> sutProvider, Guid deviceId, Guid userId, Guid identifier)
+    {
+        var notificationHubClient = Substitute.For<INotificationHubClient>();
+        sutProvider.GetDependency<INotificationHubPool>().ClientFor(Arg.Any<Guid>()).Returns(notificationHubClient);
+
+        var pushToken = "test push token";
+
+        await sutProvider.Sut.CreateOrUpdateRegistrationAsync(pushToken, deviceId.ToString(), userId.ToString(),
+            identifierNull ? null : identifier.ToString(), DeviceType.iOS);
+
+        sutProvider.GetDependency<INotificationHubPool>()
+            .Received(1)
+            .ClientFor(deviceId);
+        await notificationHubClient
+            .Received(1)
+            .CreateOrUpdateInstallationAsync(Arg.Is<Installation>(installation =>
+                installation.InstallationId == deviceId.ToString() &&
+                installation.PushChannel == pushToken &&
+                installation.Platform == NotificationPlatform.Apns &&
+                installation.Tags.Count == (identifierNull ? 2 : 3) &&
+                installation.Tags.Contains($"userId:{userId}") &&
+                installation.Tags.Contains("clientType:Mobile") &&
+                (identifierNull || installation.Tags.Contains($"deviceIdentifier:{identifier}")) &&
+                installation.Templates.Count == 3));
+        await notificationHubClient
+            .Received(1)
+            .CreateOrUpdateInstallationAsync(Arg.Is<Installation>(installation => MatchingInstallationTemplate(
+                installation.Templates, "template:payload",
+                "{\"data\":{\"type\":\"#(type)\",\"payload\":\"$(payload)\"},\"aps\":{\"content-available\":1}}",
+                new List<string?>
+                {
+                    "template:payload",
+                    $"template:payload_userId:{userId}",
+                    "clientType:Mobile",
+                    identifierNull ? null : $"template:payload_deviceIdentifier:{identifier}"
+                })));
+        await notificationHubClient
+            .Received(1)
+            .CreateOrUpdateInstallationAsync(Arg.Is<Installation>(installation => MatchingInstallationTemplate(
+                installation.Templates, "template:message",
+                "{\"data\":{\"type\":\"#(type)\"},\"aps\":{\"alert\":\"$(message)\",\"badge\":null,\"content-available\":1}}",
+                new List<string?>
+                {
+                    "template:message",
+                    $"template:message_userId:{userId}",
+                    "clientType:Mobile",
+                    identifierNull ? null : $"template:message_deviceIdentifier:{identifier}"
+                })));
+        await notificationHubClient
+            .Received(1)
+            .CreateOrUpdateInstallationAsync(Arg.Is<Installation>(installation => MatchingInstallationTemplate(
+                installation.Templates, "template:badgeMessage",
+                "{\"data\":{\"type\":\"#(type)\"},\"aps\":{\"alert\":\"$(message)\",\"badge\":\"#(badge)\",\"content-available\":1}}",
+                new List<string?>
+                {
+                    "template:badgeMessage",
+                    $"template:badgeMessage_userId:{userId}",
+                    "clientType:Mobile",
+                    identifierNull ? null : $"template:badgeMessage_deviceIdentifier:{identifier}"
+                })));
+    }
+
+    [Theory]
+    [BitAutoData(false)]
+    [BitAutoData(true)]
+    public async void CreateOrUpdateRegistrationAsync_DeviceTypeAndroidAmazon_InstallationCreated(bool identifierNull,
+        SutProvider<NotificationHubPushRegistrationService> sutProvider, Guid deviceId, Guid userId, Guid identifier)
+    {
+        var notificationHubClient = Substitute.For<INotificationHubClient>();
+        sutProvider.GetDependency<INotificationHubPool>().ClientFor(Arg.Any<Guid>()).Returns(notificationHubClient);
+
+        var pushToken = "test push token";
+
+        await sutProvider.Sut.CreateOrUpdateRegistrationAsync(pushToken, deviceId.ToString(), userId.ToString(),
+            identifierNull ? null : identifier.ToString(), DeviceType.AndroidAmazon);
+
+        sutProvider.GetDependency<INotificationHubPool>()
+            .Received(1)
+            .ClientFor(deviceId);
+        await notificationHubClient
+            .Received(1)
+            .CreateOrUpdateInstallationAsync(Arg.Is<Installation>(installation =>
+                installation.InstallationId == deviceId.ToString() &&
+                installation.PushChannel == pushToken &&
+                installation.Platform == NotificationPlatform.Adm &&
+                installation.Tags.Count == (identifierNull ? 2 : 3) &&
+                installation.Tags.Contains($"userId:{userId}") &&
+                installation.Tags.Contains("clientType:Mobile") &&
+                (identifierNull || installation.Tags.Contains($"deviceIdentifier:{identifier}")) &&
+                installation.Templates.Count == 3));
+        await notificationHubClient
+            .Received(1)
+            .CreateOrUpdateInstallationAsync(Arg.Is<Installation>(installation => MatchingInstallationTemplate(
+                installation.Templates, "template:payload",
+                "{\"data\":{\"type\":\"#(type)\",\"payload\":\"$(payload)\"}}",
+                new List<string?>
+                {
+                    "template:payload",
+                    $"template:payload_userId:{userId}",
+                    "clientType:Mobile",
+                    identifierNull ? null : $"template:payload_deviceIdentifier:{identifier}"
+                })));
+        await notificationHubClient
+            .Received(1)
+            .CreateOrUpdateInstallationAsync(Arg.Is<Installation>(installation => MatchingInstallationTemplate(
+                installation.Templates, "template:message",
+                "{\"data\":{\"type\":\"#(type)\",\"message\":\"$(message)\"}}",
+                new List<string?>
+                {
+                    "template:message",
+                    $"template:message_userId:{userId}",
+                    "clientType:Mobile",
+                    identifierNull ? null : $"template:message_deviceIdentifier:{identifier}"
+                })));
+        await notificationHubClient
+            .Received(1)
+            .CreateOrUpdateInstallationAsync(Arg.Is<Installation>(installation => MatchingInstallationTemplate(
+                installation.Templates, "template:badgeMessage",
+                "{\"data\":{\"type\":\"#(type)\",\"message\":\"$(message)\"}}",
+                new List<string?>
+                {
+                    "template:badgeMessage",
+                    $"template:badgeMessage_userId:{userId}",
+                    "clientType:Mobile",
+                    identifierNull ? null : $"template:badgeMessage_deviceIdentifier:{identifier}"
+                })));
+    }
+
+    [Theory]
+    [BitAutoData(DeviceType.ChromeBrowser)]
+    [BitAutoData(DeviceType.ChromeExtension)]
+    [BitAutoData(DeviceType.MacOsDesktop)]
+    public async void CreateOrUpdateRegistrationAsync_DeviceTypeNotMobile_InstallationCreated(DeviceType deviceType,
+        SutProvider<NotificationHubPushRegistrationService> sutProvider, Guid deviceId, Guid userId, Guid identifier)
+    {
+        var notificationHubClient = Substitute.For<INotificationHubClient>();
+        sutProvider.GetDependency<INotificationHubPool>().ClientFor(Arg.Any<Guid>()).Returns(notificationHubClient);
+
+        var pushToken = "test push token";
+
+        await sutProvider.Sut.CreateOrUpdateRegistrationAsync(pushToken, deviceId.ToString(), userId.ToString(),
+            identifier.ToString(), deviceType);
+
+        sutProvider.GetDependency<INotificationHubPool>()
+            .Received(1)
+            .ClientFor(deviceId);
+        await notificationHubClient
+            .Received(1)
+            .CreateOrUpdateInstallationAsync(Arg.Is<Installation>(installation =>
+                installation.InstallationId == deviceId.ToString() &&
+                installation.PushChannel == pushToken &&
+                installation.Tags.Count == 3 &&
+                installation.Tags.Contains($"userId:{userId}") &&
+                installation.Tags.Contains($"clientType:{DeviceTypes.ToClientType(deviceType)}") &&
+                installation.Tags.Contains($"deviceIdentifier:{identifier}") &&
+                installation.Templates.Count == 0));
+    }
+
+    private static bool MatchingInstallationTemplate(IDictionary<string, InstallationTemplate> templates, string key,
+        string body, List<string?> tags)
+    {
+        var tagsNoNulls = tags.FindAll(tag => tag != null);
+        return templates.ContainsKey(key) && templates[key].Body == body &&
+               templates[key].Tags.Count == tagsNoNulls.Count &&
+               templates[key].Tags.All(tagsNoNulls.Contains);
     }
 }