Don't attempt to send emails if they have no recipients (#6637)

NuGet · Nov 9, 2018 · 9855ce7 · 9855ce7
1 parent 5091aa4
commit 9855ce7
Show file tree

Hide file tree

Showing 31 changed files with 87 additions and 150 deletions.
diff --git a/src/NuGetGallery.Core/NuGetGallery.Core.csproj b/src/NuGetGallery.Core/NuGetGallery.Core.csproj
@@ -202,11 +202,8 @@
     <EmbeddedResource Include="Infrastructure\MigrateUserToOrganization.sql" />
   </ItemGroup>
   <ItemGroup>
-    <PackageReference Include="NuGet.Services.Entities">
-      <Version>2.32.0-agr-license-2170862</Version>
-    </PackageReference>
-    <PackageReference Include="NuGet.Services.Messaging">
-      <Version>2.31.0</Version>
+    <PackageReference Include="NuGet.Services.Messaging.Email">
+      <Version>2.36.0</Version>
     </PackageReference>
     <PackageReference Include="NuGet.StrongName.AnglicanGeek.MarkdownMailer">
       <Version>1.2.0</Version>
@@ -241,12 +238,6 @@
     <PackageReference Include="NuGet.Packaging">
       <Version>5.0.0-preview1.5665</Version>
     </PackageReference>
-    <PackageReference Include="NuGet.Services.Validation">
-      <Version>2.36.0</Version>
-    </PackageReference>
-    <PackageReference Include="NuGet.Services.Validation.Issues">
-      <Version>2.36.0</Version>
-    </PackageReference>
     <PackageReference Include="WindowsAzure.Storage">
       <Version>7.1.2</Version>
     </PackageReference>

diff --git a/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs b/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs
@@ -58,6 +58,15 @@ public static class BindingKeys
 
         protected override void Load(ContainerBuilder builder)
         {
+            var loggerConfiguration = LoggingSetup.CreateDefaultLoggerConfiguration(withConsoleLogger: false);
+            var loggerFactory = LoggingSetup.CreateLoggerFactory(loggerConfiguration);
+            builder.RegisterInstance(loggerFactory)
+                .AsSelf()
+                .As<ILoggerFactory>();
+            builder.RegisterGeneric(typeof(Logger<>))
+                .As(typeof(ILogger<>))
+                .SingleInstance();
+
             var telemetryClient = TelemetryClientWrapper.Instance;
             builder.RegisterInstance(telemetryClient)
                 .AsSelf()
@@ -466,18 +475,11 @@ private static void RegisterAsynchronousEmailMessagingService(ContainerBuilder b
                 .Keyed<ITopicClient>(BindingKeys.EmailPublisherTopic)
                 .OnRelease(x => x.Close());
 
-            // Create an ILoggerFactory
-            var loggerConfiguration = LoggingSetup.CreateDefaultLoggerConfiguration(withConsoleLogger: false);
-            var loggerFactory = LoggingSetup.CreateLoggerFactory(loggerConfiguration);
-
             builder
                 .RegisterType<EmailMessageEnqueuer>()
                 .WithParameter(new ResolvedParameter(
                     (pi, ctx) => pi.ParameterType == typeof(ITopicClient),
                     (pi, ctx) => ctx.ResolveKeyed<ITopicClient>(BindingKeys.EmailPublisherTopic)))
-                .WithParameter(new ResolvedParameter(
-                    (pi, ctx) => pi.ParameterType == typeof(ILogger<EmailMessageEnqueuer>),
-                    (pi, ctx) => loggerFactory.CreateLogger<EmailMessageEnqueuer>()))
                 .As<IEmailMessageEnqueuer>();
 
             builder.RegisterType<AsynchronousEmailMessageService>()

diff --git a/...ure/Mail/EmailRecipientsWithPermission.cs → ...ure/Mail/GalleryEmailRecipientsUtility.cs b/...ure/Mail/EmailRecipientsWithPermission.cs → ...ure/Mail/GalleryEmailRecipientsUtility.cs
@@ -6,19 +6,12 @@
 using System.Linq;
 using System.Net.Mail;
 using NuGet.Services.Entities;
-using NuGet.Services.Messaging.Email;
 
 namespace NuGetGallery.Infrastructure.Mail
 {
-    public class EmailRecipientsWithPermission
-        : IEmailRecipients
+    public static class GalleryEmailRecipientsUtility
     {
-        public EmailRecipientsWithPermission(
-            User user,
-            ActionRequiringAccountPermissions action,
-            IReadOnlyList<MailAddress> cc = null,
-            IReadOnlyList<MailAddress> bcc = null,
-            IReadOnlyList<MailAddress> replyTo = null)
+        public static IReadOnlyList<MailAddress> GetAddressesWithPermission(User user, ActionRequiringAccountPermissions action)
         {
             if (user == null)
             {
@@ -30,23 +23,6 @@ public EmailRecipientsWithPermission(
                 throw new ArgumentNullException(nameof(action));
             }
 
-            To = AddAddressesWithPermission(user, action);
-
-            CC = cc ?? new List<MailAddress>();
-            Bcc = bcc ?? new List<MailAddress>();
-            ReplyTo = replyTo ?? new List<MailAddress>();
-        }
-
-        public IReadOnlyList<MailAddress> To { get; }
-
-        public IReadOnlyList<MailAddress> CC { get; }
-
-        public IReadOnlyList<MailAddress> Bcc { get; }
-
-        public IReadOnlyList<MailAddress> ReplyTo { get; }
-
-        private static IReadOnlyList<MailAddress> AddAddressesWithPermission(User user, ActionRequiringAccountPermissions action)
-        {
             var recipients = new List<MailAddress>();
 
             if (user is Organization organization)

diff --git a/src/NuGetGallery/Infrastructure/Mail/Messages/ContactOwnersMessage.cs b/src/NuGetGallery/Infrastructure/Mail/Messages/ContactOwnersMessage.cs
@@ -39,12 +39,10 @@ public ContactOwnersMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            var to = EmailRecipients.GetAllOwners(
-                Package.PackageRegistration,
-                requireEmailAllowed: true);
-
             return new EmailRecipients(
-                to,
+                to: EmailRecipients.GetAllOwners(
+                    Package.PackageRegistration,
+                    requireEmailAllowed: true),
                 replyTo: new[] { FromAddress });
         }
 

diff --git a/src/NuGetGallery/Infrastructure/Mail/Messages/OrganizationMemberRemovedMessage.cs b/src/NuGetGallery/Infrastructure/Mail/Messages/OrganizationMemberRemovedMessage.cs
@@ -31,13 +31,10 @@ public OrganizationMemberRemovedMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            if (!Organization.EmailAllowed)
-            {
-                return EmailRecipients.None;
-            }
-
             return new EmailRecipients(
-                to: new[] { Organization.ToMailAddress() },
+                to: Organization.EmailAllowed
+                    ? new[] { Organization.ToMailAddress() }
+                    : new MailAddress[0],
                 replyTo: new[] { RemovedUser.ToMailAddress() });
         }
 

diff --git a/src/NuGetGallery/Infrastructure/Mail/Messages/OrganizationMemberUpdatedMessage.cs b/src/NuGetGallery/Infrastructure/Mail/Messages/OrganizationMemberUpdatedMessage.cs
@@ -31,13 +31,10 @@ public OrganizationMemberUpdatedMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            if (!Organization.EmailAllowed)
-            {
-                return EmailRecipients.None;
-            }
-
             return new EmailRecipients(
-                to: new[] { Organization.ToMailAddress() },
+                to: Organization.EmailAllowed
+                    ? new[] { Organization.ToMailAddress() }
+                    : new MailAddress[0],
                 replyTo: new[] { Membership.Member.ToMailAddress() });
         }
 

diff --git a/...NuGetGallery/Infrastructure/Mail/Messages/OrganizationMembershipRequestCanceledMessage.cs b/...NuGetGallery/Infrastructure/Mail/Messages/OrganizationMembershipRequestCanceledMessage.cs
@@ -31,13 +31,10 @@ public OrganizationMembershipRequestCanceledMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            if (!PendingUser.EmailAllowed)
-            {
-                return EmailRecipients.None;
-            }
-
             return new EmailRecipients(
-                to: new[] { PendingUser.ToMailAddress() },
+                to: PendingUser.EmailAllowed
+                    ? new[] { PendingUser.ToMailAddress() }
+                    : new MailAddress[0],
                 replyTo: new[] { Organization.ToMailAddress() });
         }
 

diff --git a/...NuGetGallery/Infrastructure/Mail/Messages/OrganizationMembershipRequestDeclinedMessage.cs b/...NuGetGallery/Infrastructure/Mail/Messages/OrganizationMembershipRequestDeclinedMessage.cs
@@ -31,9 +31,10 @@ public OrganizationMembershipRequestDeclinedMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            return new EmailRecipientsWithPermission(
-                Organization,
-                ActionsRequiringPermissions.ManageAccount,
+            return new EmailRecipients(
+                to: GalleryEmailRecipientsUtility.GetAddressesWithPermission(
+                    Organization,
+                    ActionsRequiringPermissions.ManageAccount),
                 replyTo: new[] { PendingUser.ToMailAddress() });
         }
 

diff --git a/...uGetGallery/Infrastructure/Mail/Messages/OrganizationMembershipRequestInitiatedMessage.cs b/...uGetGallery/Infrastructure/Mail/Messages/OrganizationMembershipRequestInitiatedMessage.cs
@@ -43,9 +43,10 @@ public OrganizationMembershipRequestInitiatedMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            return new EmailRecipientsWithPermission(
-                Organization,
-                ActionsRequiringPermissions.ManageAccount,
+            return new EmailRecipients(
+                to: GalleryEmailRecipientsUtility.GetAddressesWithPermission(
+                    Organization,
+                    ActionsRequiringPermissions.ManageAccount),
                 replyTo: new[] { RequestingUser.ToMailAddress() });
         }
 

diff --git a/src/NuGetGallery/Infrastructure/Mail/Messages/OrganizationMembershipRequestMessage.cs b/src/NuGetGallery/Infrastructure/Mail/Messages/OrganizationMembershipRequestMessage.cs
@@ -50,13 +50,10 @@ public OrganizationMembershipRequestMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            if (!NewUser.EmailAllowed)
-            {
-                return EmailRecipients.None;
-            }
-
             return new EmailRecipients(
-                to: new[] { NewUser.ToMailAddress() },
+                to: NewUser.EmailAllowed
+                    ? new[] { NewUser.ToMailAddress() }
+                    : new MailAddress[0],
                 replyTo: new[]
                 {
                     Organization.ToMailAddress(),

diff --git a/src/NuGetGallery/Infrastructure/Mail/Messages/OrganizationTransformAcceptedMessage.cs b/src/NuGetGallery/Infrastructure/Mail/Messages/OrganizationTransformAcceptedMessage.cs
@@ -29,13 +29,10 @@ public OrganizationTransformAcceptedMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            if (!_accountToTransform.EmailAllowed)
-            {
-                return EmailRecipients.None;
-            }
-
             return new EmailRecipients(
-                to: new[] { _accountToTransform.ToMailAddress() },
+                to: _accountToTransform.EmailAllowed
+                    ? new[] { _accountToTransform.ToMailAddress() }
+                    : new MailAddress[0],
                 replyTo: new[] { _adminUser.ToMailAddress() });
         }
 

diff --git a/src/NuGetGallery/Infrastructure/Mail/Messages/OrganizationTransformInitiatedMessage.cs b/src/NuGetGallery/Infrastructure/Mail/Messages/OrganizationTransformInitiatedMessage.cs
@@ -35,13 +35,10 @@ public OrganizationTransformInitiatedMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            if (!_accountToTransform.EmailAllowed)
-            {
-                return EmailRecipients.None;
-            }
-
             return new EmailRecipients(
-                to: new[] { _accountToTransform.ToMailAddress() },
+                to: _accountToTransform.EmailAllowed
+                    ? new[] { _accountToTransform.ToMailAddress() }
+                    : new MailAddress[0],
                 replyTo: new[] { _adminUser.ToMailAddress() });
         }
 

diff --git a/src/NuGetGallery/Infrastructure/Mail/Messages/OrganizationTransformRejectedMessage.cs b/src/NuGetGallery/Infrastructure/Mail/Messages/OrganizationTransformRejectedMessage.cs
@@ -43,13 +43,10 @@ public OrganizationTransformRejectedMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            if (!AccountToSendTo.EmailAllowed)
-            {
-                return EmailRecipients.None;
-            }
-
             return new EmailRecipients(
-                to: new[] { AccountToSendTo.ToMailAddress() },
+                to: AccountToSendTo.EmailAllowed
+                    ? new[] { AccountToSendTo.ToMailAddress() }
+                    : new MailAddress[0],
                 replyTo: new[] { AccountToReplyTo.ToMailAddress() });
         }
 

diff --git a/src/NuGetGallery/Infrastructure/Mail/Messages/OrganizationTransformRequestMessage.cs b/src/NuGetGallery/Infrastructure/Mail/Messages/OrganizationTransformRequestMessage.cs
@@ -44,13 +44,10 @@ public OrganizationTransformRequestMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            if (!_adminUser.EmailAllowed)
-            {
-                return EmailRecipients.None;
-            }
-
             return new EmailRecipients(
-                to: new[] { _adminUser.ToMailAddress() },
+                to: _adminUser.EmailAllowed
+                    ? new[] { _adminUser.ToMailAddress() }
+                    : new MailAddress[0],
                 replyTo: new[] { _accountToTransform.ToMailAddress() });
         }
 

diff --git a/src/NuGetGallery/Infrastructure/Mail/Messages/PackageDeletedNoticeMessage.cs b/src/NuGetGallery/Infrastructure/Mail/Messages/PackageDeletedNoticeMessage.cs
@@ -33,10 +33,10 @@ public PackageDeletedNoticeMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            var to = EmailRecipients.GetAllOwners(
-                Package.PackageRegistration,
-                requireEmailAllowed: false);
-            return new EmailRecipients(to);
+            return new EmailRecipients(
+                to: EmailRecipients.GetAllOwners(
+                    Package.PackageRegistration,
+                    requireEmailAllowed: false));
         }
 
         public override string GetSubject()

diff --git a/src/NuGetGallery/Infrastructure/Mail/Messages/PackageOwnerAddedMessage.cs b/src/NuGetGallery/Infrastructure/Mail/Messages/PackageOwnerAddedMessage.cs
@@ -39,9 +39,10 @@ public PackageOwnerAddedMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            return new EmailRecipientsWithPermission(
-                ToUser,
-                ActionsRequiringPermissions.HandlePackageOwnershipRequest,
+            return new EmailRecipients(
+                to: GalleryEmailRecipientsUtility.GetAddressesWithPermission(
+                    ToUser,
+                    ActionsRequiringPermissions.HandlePackageOwnershipRequest),
                 replyTo: new[] { _configuration.GalleryNoReplyAddress });
         }
 

diff --git a/src/NuGetGallery/Infrastructure/Mail/Messages/PackageOwnerRemovedMessage.cs b/src/NuGetGallery/Infrastructure/Mail/Messages/PackageOwnerRemovedMessage.cs
@@ -34,9 +34,10 @@ public PackageOwnerRemovedMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            return new EmailRecipientsWithPermission(
-                ToUser,
-                ActionsRequiringPermissions.HandlePackageOwnershipRequest,
+            return new EmailRecipients(
+                to: GalleryEmailRecipientsUtility.GetAddressesWithPermission(
+                    ToUser,
+                    ActionsRequiringPermissions.HandlePackageOwnershipRequest),
                 replyTo: new[] { FromUser.ToMailAddress() });
         }
 

diff --git a/src/NuGetGallery/Infrastructure/Mail/Messages/PackageOwnershipRequestCanceledMessage.cs b/src/NuGetGallery/Infrastructure/Mail/Messages/PackageOwnershipRequestCanceledMessage.cs
@@ -35,9 +35,10 @@ public PackageOwnershipRequestCanceledMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            return new EmailRecipientsWithPermission(
-                NewOwner,
-                ActionsRequiringPermissions.HandlePackageOwnershipRequest,
+            return new EmailRecipients(
+                to: GalleryEmailRecipientsUtility.GetAddressesWithPermission(
+                    NewOwner,
+                    ActionsRequiringPermissions.HandlePackageOwnershipRequest),
                 replyTo: new[] { RequestingOwner.ToMailAddress() });
         }
 

diff --git a/src/NuGetGallery/Infrastructure/Mail/Messages/PackageOwnershipRequestDeclinedMessage.cs b/src/NuGetGallery/Infrastructure/Mail/Messages/PackageOwnershipRequestDeclinedMessage.cs
@@ -35,15 +35,11 @@ public PackageOwnershipRequestDeclinedMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            if (!RequestingOwner.EmailAllowed)
-            {
-                // This will prevent the email from being sent.
-                return EmailRecipients.None;
-            }
-
-            return new EmailRecipientsWithPermission(
-                RequestingOwner,
-                ActionsRequiringPermissions.HandlePackageOwnershipRequest,
+            return new EmailRecipients(
+                to: RequestingOwner.EmailAllowed
+                    ? GalleryEmailRecipientsUtility.GetAddressesWithPermission(
+                        RequestingOwner, ActionsRequiringPermissions.HandlePackageOwnershipRequest)
+                    : new MailAddress[0],
                 replyTo: new[] { NewOwner.ToMailAddress() });
         }
 

diff --git a/src/NuGetGallery/Infrastructure/Mail/Messages/PackageOwnershipRequestInitiatedMessage.cs b/src/NuGetGallery/Infrastructure/Mail/Messages/PackageOwnershipRequestInitiatedMessage.cs
@@ -46,9 +46,10 @@ public PackageOwnershipRequestInitiatedMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            return new EmailRecipientsWithPermission(
-                ReceivingOwner,
-                ActionsRequiringPermissions.HandlePackageOwnershipRequest,
+            return new EmailRecipients(
+                to: GalleryEmailRecipientsUtility.GetAddressesWithPermission(
+                    ReceivingOwner,
+                    ActionsRequiringPermissions.HandlePackageOwnershipRequest),
                 replyTo: new[] { NewOwner.ToMailAddress() });
         }
 

diff --git a/src/NuGetGallery/Infrastructure/Mail/Messages/PackageOwnershipRequestMessage.cs b/src/NuGetGallery/Infrastructure/Mail/Messages/PackageOwnershipRequestMessage.cs
@@ -55,9 +55,10 @@ public PackageOwnershipRequestMessage(
 
         public override IEmailRecipients GetRecipients()
         {
-            return new EmailRecipientsWithPermission(
-                ToUser,
-                ActionsRequiringPermissions.HandlePackageOwnershipRequest,
+            return new EmailRecipients(
+                to: GalleryEmailRecipientsUtility.GetAddressesWithPermission(
+                    ToUser,
+                    ActionsRequiringPermissions.HandlePackageOwnershipRequest),
                 replyTo: new[] { FromUser.ToMailAddress() });
         }