Merge pull request #460 from NuGet/dev

[ReleasePrep][06.01.2018]RI of dev into master

Author: shishirx34

NuGet.Jobs.sln

@@ -129,6 +129,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Monitoring", "Monitoring",
 EndProject
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Monitoring.PackageLag", "src\PackageLagMonitor\Monitoring.PackageLag.csproj", "{B5147169-E941-4CF8-9FCD-1C123ACD3149}"
 EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Tests.CredentialExpiration", "tests\Tests.CredentialExpiration\Tests.CredentialExpiration.csproj", "{60152AB1-2EB4-4D44-B6D6-EEE24209A1F7}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -333,6 +335,10 @@ Global
 		{B5147169-E941-4CF8-9FCD-1C123ACD3149}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{B5147169-E941-4CF8-9FCD-1C123ACD3149}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{B5147169-E941-4CF8-9FCD-1C123ACD3149}.Release|Any CPU.Build.0 = Release|Any CPU
+		{60152AB1-2EB4-4D44-B6D6-EEE24209A1F7}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{60152AB1-2EB4-4D44-B6D6-EEE24209A1F7}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{60152AB1-2EB4-4D44-B6D6-EEE24209A1F7}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{60152AB1-2EB4-4D44-B6D6-EEE24209A1F7}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -387,6 +393,7 @@ Global
 		{136411AF-B9FA-438D-B790-9FB78A5F7F54} = {6A776396-02B1-475D-A104-26940ADB04AB}
 		{CAE45AC9-F11B-4215-9D1A-C98BC0F1F687} = {6A776396-02B1-475D-A104-26940ADB04AB}
 		{B5147169-E941-4CF8-9FCD-1C123ACD3149} = {814F9B31-4AF3-46CC-AD61-CEB40F47083A}
+		{60152AB1-2EB4-4D44-B6D6-EEE24209A1F7} = {6A776396-02B1-475D-A104-26940ADB04AB}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {284A7AC3-FB43-4F1F-9C9C-2AF0E1F46C2B}

src/Gallery.CredentialExpiration/Gallery.CredentialExpiration.csproj

@@ -45,7 +45,11 @@
     <Reference Include="System.Xml" />
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="GalleryCredentialExpiration.cs" />
+    <Compile Include="ICredentialExpirationExporter.cs" />
+    <Compile Include="JobRunTimeCursor.cs" />
     <Compile Include="LogEvents.cs" />
+    <Compile Include="Models\CredentialExpirationJobMetadata.cs" />
     <Compile Include="MyJobArgumentNames.cs" />
     <Compile Include="Models\ExpiredCredentialData.cs" />
     <Compile Include="Job.cs" />

src/Gallery.CredentialExpiration/GalleryCredentialExpiration.cs

@@ -0,0 +1,101 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using System.Data.SqlClient;
+using System.Linq;
+using System.Threading.Tasks;
+using NuGet.Services.Sql;
+using Gallery.CredentialExpiration.Models;
+
+namespace Gallery.CredentialExpiration
+{
+    public class GalleryCredentialExpiration : ICredentialExpirationExporter
+    {
+        private readonly CredentialExpirationJobMetadata _jobMetadata;
+        private readonly ISqlConnectionFactory _galleryDatabase;
+
+        public GalleryCredentialExpiration(CredentialExpirationJobMetadata jobMetadata, ISqlConnectionFactory galleryDatabase)
+        {
+            _jobMetadata = jobMetadata;
+            _galleryDatabase = galleryDatabase;
+        }
+
+        /// <summary>
+        /// Used for the expiring credentials.
+        /// </summary>
+        /// <param name="jobMetadata"></param>
+        /// <returns></returns>
+        public DateTimeOffset GetMaxNotificationDate()
+        {
+            return _jobMetadata.JobRunTime.AddDays(_jobMetadata.WarnDaysBeforeExpiration);
+        }
+
+        /// <summary>
+        /// Used for the Expired credentials.
+        /// </summary>
+        /// <param name="jobMetadata"></param>
+        /// <returns></returns>
+        public DateTimeOffset GetMinNotificationDate()
+        {
+            // In case that the job failed to run for more than 1 day, go back more than the WarnDaysBeforeExpiration value 
+            // with the number of days that the job did not run
+            return  _jobMetadata.JobCursor.JobCursorTime;
+        }
+
+        public async Task<List<ExpiredCredentialData>> GetCredentialsAsync(TimeSpan timeout)
+        {
+            // Set the day interval for the accounts that will be queried for expiring /expired credentials.
+            var maxNotificationDate = ConvertToString(GetMaxNotificationDate());
+            var minNotificationDate = ConvertToString(GetMinNotificationDate());
+
+            // Connect to database
+            using (var galleryConnection = await _galleryDatabase.CreateAsync())
+            {
+                // Fetch credentials that expire in _warnDaysBeforeExpiration days 
+                // + the user's e-mail address
+                return  (await galleryConnection.QueryWithRetryAsync<ExpiredCredentialData>(
+                    Strings.GetExpiredCredentialsQuery,
+                    param: new { MaxNotificationDate = maxNotificationDate, MinNotificationDate = minNotificationDate },
+                    maxRetries: 3,
+                    commandTimeout: timeout)).ToList();
+            }
+        }
+
+        /// <summary>
+        /// Send email of credential expired during the time interval [_jobMetadata.CursorTime, _jobMetadata.JobRunTime) 
+        /// </summary>
+        /// <param name="credentialSet"></param>
+        /// <returns></returns>
+        public List<ExpiredCredentialData> GetExpiredCredentials(List<ExpiredCredentialData> credentialSet)
+        {
+            // Send email to the accounts that had credentials expired from the last execution.
+            // The second condition is meant only for far cases that the SQL query for data filtering was modified by mistake and more credentials were included.
+            return credentialSet.Where(x => (x.Expires < _jobMetadata.JobRunTime) && (x.Expires >= _jobMetadata.JobCursor.JobCursorTime)).ToList();
+        }
+
+        /// <summary>
+        /// Returns the expiring credentials.
+        /// </summary>
+        /// <param name="credentialSet"></param>
+        /// <returns></returns>
+        public List<ExpiredCredentialData> GetExpiringCredentials(List<ExpiredCredentialData> credentialSet)
+        {
+            // Send email to the accounts that will have credentials expiring in the next _warnDaysBeforeExpiration days and did not have any warning email sent yet.
+            // Avoid cases when the cursor is out of date and MaxProcessedCredentialsTime < JobRuntime
+            var sendEmailsDateLeftBoundary = (_jobMetadata.JobCursor.MaxProcessedCredentialsTime > _jobMetadata.JobRunTime) ? _jobMetadata.JobCursor.MaxProcessedCredentialsTime : _jobMetadata.JobRunTime;
+            return credentialSet.Where( x => x.Expires > sendEmailsDateLeftBoundary).ToList();
+        }
+
+        /// <summary>
+        /// Converts a <see cref="DateTimeOffset"/> string with the "yyyy-MM-dd HH:mm:ss" format.
+        /// </summary>
+        /// <param name="value">The <see cref="DateTimeOffset"/> to be converterd.</param>
+        /// <returns></returns>
+        private string ConvertToString(DateTimeOffset value)
+        {
+            return value.ToString("yyyy-MM-dd HH:mm:ss");
+        }
+    }
+}

src/Gallery.CredentialExpiration/ICredentialExpirationExporter.cs

@@ -0,0 +1,34 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using System.Threading.Tasks;
+using Gallery.CredentialExpiration.Models;
+
+namespace Gallery.CredentialExpiration
+{
+    public interface ICredentialExpirationExporter
+    {
+        /// <summary>
+        /// Returns the entire credential set to work with.
+        /// </summary>
+        /// <param name="credentialSet"></param>
+        /// <returns></returns>
+        Task<List<ExpiredCredentialData>> GetCredentialsAsync(TimeSpan timeout);
+
+        /// <summary>
+        /// Returns the set of expired credentials that will have notification emails sent.
+        /// </summary>
+        /// <param name="credentialSet"></param>
+        /// <returns></returns>
+        List<ExpiredCredentialData> GetExpiredCredentials(List<ExpiredCredentialData> credentialSet);
+
+        /// <summary>
+        /// Returns the set of expiring credentials that will have notification emails sent.
+        /// </summary>
+        /// <param name="credentialSet"></param>
+        /// <returns></returns>
+        List<ExpiredCredentialData> GetExpiringCredentials(List<ExpiredCredentialData> credentialSet);
+    }
+}

src/Gallery.CredentialExpiration/Job.cs

@@ -15,6 +15,7 @@
 using Microsoft.Extensions.Logging;
 using Microsoft.WindowsAzure.Storage;
 using Newtonsoft.Json;
+using Newtonsoft.Json.Linq;
 using NuGet.Jobs;
 using NuGet.Services.KeyVault;
 using NuGet.Services.Sql;
@@ -26,8 +27,7 @@ public class Job : JobBase
     {
         private readonly TimeSpan _defaultCommandTimeout = TimeSpan.FromMinutes(30);
 
-        private readonly ConcurrentDictionary<string, DateTimeOffset> _contactedUsers = new ConcurrentDictionary<string, DateTimeOffset>();
-        private readonly string _cursorFile = "cursor.json";
+        private readonly string _cursorFile = "cursorv2.json";
 
         private bool _whatIf = false;
 
@@ -39,7 +39,6 @@ public class Job : JobBase
         private string _mailFrom;
         private SmtpClient _smtpClient;
 
-        private int _allowEmailResendAfterDays = 7;
         private int _warnDaysBeforeExpiration = 10;
 
         private Storage _storage;
@@ -64,9 +63,6 @@ public override void Init(IServiceContainer serviceContainer, IDictionary<string
             _warnDaysBeforeExpiration = JobConfigurationManager.TryGetIntArgument(jobArgsDictionary, MyJobArgumentNames.WarnDaysBeforeExpiration)
                 ?? _warnDaysBeforeExpiration;
 
-            _allowEmailResendAfterDays = JobConfigurationManager.TryGetIntArgument(jobArgsDictionary, MyJobArgumentNames.AllowEmailResendAfterDays)
-                ?? _allowEmailResendAfterDays;
-
             var storageConnectionString = JobConfigurationManager.GetArgument(jobArgsDictionary, JobArgumentNames.DataStorageAccount);
             var storageContainerName = JobConfigurationManager.GetArgument(jobArgsDictionary, JobArgumentNames.ContainerName);
 
@@ -77,89 +73,61 @@ public override void Init(IServiceContainer serviceContainer, IDictionary<string
 
         public override async Task Run()
         {
+            var jobRunTime = DateTimeOffset.UtcNow;
+            // Default values
+            var jobCursor = new JobRunTimeCursor( jobCursorTime: jobRunTime, maxProcessedCredentialsTime: jobRunTime );
+            var galleryCredentialExpiration = new GalleryCredentialExpiration(new CredentialExpirationJobMetadata(jobRunTime, _warnDaysBeforeExpiration, jobCursor), _galleryDatabase);
+
             try
             {
-                List<ExpiredCredentialData> expiredCredentials = null;
+                List<ExpiredCredentialData> credentialsInRange = null;
 
-                // Who did we contact before?
+                // Get the most recent date for the emails being sent 
                 if (_storage.Exists(_cursorFile))
                 {
                     string content = await _storage.LoadString(_storage.ResolveUri(_cursorFile), CancellationToken.None);
                     // Load from cursor
-                    var contactedUsers = JsonConvert.DeserializeObject<Dictionary<string, DateTimeOffset>>(content);
-
-                    // Clean older entries (contacted in last _allowEmailResendAfterDays)
-                    var referenceDate = DateTimeOffset.UtcNow.AddDays(-1 * _allowEmailResendAfterDays);
-                    foreach (var kvp in contactedUsers.Where(kvp => kvp.Value >= referenceDate))
-                    {
-                        _contactedUsers.AddOrUpdate(kvp.Key, kvp.Value, (s, offset) => kvp.Value);
-                    }
+                    // Throw if the schema is not correct to ensure that not-intended emails are sent.
+                    jobCursor = JsonConvert.DeserializeObject<JobRunTimeCursor>(content, new JsonSerializerSettings() { MissingMemberHandling = MissingMemberHandling.Error });
+                    galleryCredentialExpiration = new GalleryCredentialExpiration(new CredentialExpirationJobMetadata(jobRunTime, _warnDaysBeforeExpiration, jobCursor), _galleryDatabase);
                 }
 
                 // Connect to database
-                using (var galleryConnection = await _galleryDatabase.CreateAsync())
-                {
-                    // Fetch credentials that expire in _warnDaysBeforeExpiration days 
-                    // + the user's e-mail address
-                    Logger.LogInformation("Retrieving expired credentials from Gallery database...");
-
-                    expiredCredentials = (await galleryConnection.QueryWithRetryAsync<ExpiredCredentialData>(
-                        Strings.GetExpiredCredentialsQuery,
-                        param: new { DaysBeforeExpiration = _warnDaysBeforeExpiration },
-                        maxRetries: 3,
-                        commandTimeout: _defaultCommandTimeout)).ToList();
-
-                    Logger.LogInformation("Retrieved {ExpiredCredentials} expired credentials.",
-                        expiredCredentials.Count);
-                }
+                Logger.LogInformation("Retrieving expired credentials from Gallery database...");
+                credentialsInRange = await galleryCredentialExpiration.GetCredentialsAsync(_defaultCommandTimeout);
+                Logger.LogInformation("Retrieved {ExpiredCredentials} expired credentials.",
+                       credentialsInRange.Count);
 
                 // Add default description for non-scoped API keys
-                expiredCredentials
+                credentialsInRange
                     .Where(cred => string.IsNullOrEmpty(cred.Description))
                     .ToList()
                     .ForEach(ecd => ecd.Description = Constants.NonScopedApiKeyDescription);
 
                 // Group credentials for each user
-                var userToExpiredCredsMapping = expiredCredentials
+                var userToExpiredCredsMapping = credentialsInRange
                     .GroupBy(x => x.Username)
                     .ToDictionary(user => user.Key, value => value.ToList());
 
-                // Handle expiring credentials
-                var jobRunTime = DateTimeOffset.UtcNow;
                 foreach (var userCredMapping in userToExpiredCredsMapping)
                 {
                     var username = userCredMapping.Key;
                     var credentialList = userCredMapping.Value;
 
                     // Split credentials into two lists: Expired and Expiring to aggregate messages
-                    var expiringCredentialList = credentialList
-                        .Where(x => (x.Expires - jobRunTime).TotalDays > 0)
-                        .ToList();
-                    var expiredCredentialList = credentialList
-                        .Where(x => (x.Expires - jobRunTime).TotalDays <= 0)
-                        .ToList();
-
-                    DateTimeOffset userContactTime;
-                    if (!_contactedUsers.TryGetValue(username, out userContactTime))
-                    {
-                        // send expiring API keys email notification
-                        await HandleExpiredCredentialEmail(username, expiringCredentialList, jobRunTime, expired: false);
-
-                        // send expired API keys email notification
-                        await HandleExpiredCredentialEmail(username, expiredCredentialList, jobRunTime, expired: true);
-                    }
-                    else
-                    {
-                        Logger.LogDebug("Skipping expired credential for user {Username} - already handled at {JobRuntime}.",
-                            username, userContactTime);
-                    }
+                    var expiringCredentialList = galleryCredentialExpiration.GetExpiringCredentials(credentialList);
+                    var expiredCredentialList = galleryCredentialExpiration.GetExpiredCredentials(credentialList);
+
+                    await HandleExpiredCredentialEmail(username, expiringCredentialList, jobRunTime, expired: false);
+
+                    // send expired API keys email notification
+                    await HandleExpiredCredentialEmail(username, expiredCredentialList, jobRunTime, expired: true);
                 }
             }
             finally
             {
-                // Make sure we know who has been contacted today, so they do not get double
-                // e-mail notifications.
-                string json = JsonConvert.SerializeObject(_contactedUsers);
+                JobRunTimeCursor newCursor = new JobRunTimeCursor( jobCursorTime: jobRunTime, maxProcessedCredentialsTime: galleryCredentialExpiration.GetMaxNotificationDate());
+                string json = JsonConvert.SerializeObject(newCursor);
                 var content = new StringStorageContent(json, "application/json");
                 await _storage.Save(_storage.ResolveUri(_cursorFile), content, CancellationToken.None);
             }
@@ -172,9 +140,8 @@ private async Task HandleExpiredCredentialEmail(string username, List<ExpiredCre
                 return;
             }
 
-            Logger.LogInformation("Handling {Expired} credential(s) for user {Username} (Keys: {Descriptions})...",
+            Logger.LogInformation("Handling {Expired} credential(s) (Keys: {Descriptions})...",
                 expired ? "expired" : "expiring",
-                username,
                 string.Join(", ", credentialList.Select(x => x.Description).ToList()));
 
             // Build message
@@ -206,21 +173,18 @@ private async Task HandleExpiredCredentialEmail(string username, List<ExpiredCre
                     await _smtpClient.SendMailAsync(mailMessage);
                 }
 
-                Logger.LogInformation("Handled {Expired} credential for user {Username}.",
-                    expired ? "expired" : "expiring",
-                    username);
-
-                _contactedUsers.AddOrUpdate(username, jobRunTime, (s, offset) => jobRunTime);
+                Logger.LogInformation("Handled {Expired} credential .",
+                    expired ? "expired" : "expiring");
             }
             catch (SmtpFailedRecipientException ex)
             {
-                var logMessage = "Failed to handle credential for user {Username} - recipient failed!";
-                Logger.LogWarning(LogEvents.FailedToSendMail, ex, logMessage, username);
+                var logMessage = "Failed to handle credential - recipient failed!";
+                Logger.LogWarning(LogEvents.FailedToSendMail, ex, logMessage);
             }
             catch (Exception ex)
             {
-                var logMessage = "Failed to handle credential for user {Username}.";
-                Logger.LogCritical(LogEvents.FailedToHandleExpiredCredential, ex, logMessage, username);
+                var logMessage = "Failed to handle credential .";
+                Logger.LogCritical(LogEvents.FailedToHandleExpiredCredential, ex, logMessage);
 
                 throw;
             }

src/Gallery.CredentialExpiration/JobRunTimeCursor.cs

@@ -0,0 +1,20 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+
+namespace Gallery.CredentialExpiration
+{
+    public class JobRunTimeCursor
+    {
+        public JobRunTimeCursor(DateTimeOffset jobCursorTime, DateTimeOffset maxProcessedCredentialsTime)
+        {
+            JobCursorTime = jobCursorTime;
+            MaxProcessedCredentialsTime = maxProcessedCredentialsTime;
+        }
+
+        public DateTimeOffset JobCursorTime { get; }
+
+        public DateTimeOffset MaxProcessedCredentialsTime { get; }
+    }
+}