Extract Contacts class + related refactorings to NotificationConfigurator class. (#5214)

Author: Konrad Jamrozik

tools/identity-resolution/Services/GitHubService.cs

@@ -35,7 +35,7 @@ public GitHubService(ILogger<GitHubService> logger)
         /// </summary>
         /// <param name="repoUrl">GitHub repository URL</param>
         /// <returns>Contents fo the located CODEOWNERS file</returns>
-        public async Task<List<CodeownersEntry>> GetCodeownersFile(Uri repoUrl)
+        public async Task<List<CodeownersEntry>> GetCodeownersFileEntries(Uri repoUrl)
         {
             List<CodeownersEntry> result;
             if (codeownersFileCache.TryGetValue(repoUrl.ToString(), out result))
@@ -68,7 +68,7 @@ private async Task<List<CodeownersEntry>> GetCodeownersFileImpl(Uri repoUrl)
             }
 
             logger.LogWarning("Could not retrieve CODEOWNERS file URL = {0} ResponseCode = {1}", codeOwnersUrl, result.StatusCode);
-            return default;
+            return null;
         }
 
     }

tools/notification-configuration/notification-creator/Contacts.cs

@@ -0,0 +1,122 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Text.RegularExpressions;
+using System.Threading.Tasks;
+using Azure.Sdk.Tools.CodeOwnersParser;
+using Microsoft.Extensions.Logging;
+using Microsoft.TeamFoundation.Build.WebApi;
+
+namespace Azure.Sdk.Tools.NotificationConfiguration;
+
+/// <summary>
+/// This class represents a set of contacts obtained from CODEOWNERS file
+/// located in repository attached to given build definition [1].
+///
+/// The contacts are the CODEOWNERS path owners of path that matches the build definition file path.
+///
+/// To obtain the contacts, construct this class and then call GetFromBuildDefinitionRepoCodeowners(buildDefinition).
+///
+/// [1] https://learn.microsoft.com/en-us/rest/api/azure/devops/build/definitions/get?view=azure-devops-rest-7.0#builddefinition
+/// </summary>
+internal class Contacts
+{
+    private readonly ILogger log;
+    private readonly GitHubService gitHubService;
+
+    // Type 2 maps to a build definition YAML file in the repository.
+    // You can confirm it by decompiling Microsoft.TeamFoundation.Build.WebApi.YamlProcess..ctor.
+    private const int BuildDefinitionYamlProcessType = 2;
+
+    internal Contacts(GitHubService gitHubService, ILogger log)
+    {
+        this.log = log;
+        this.gitHubService = gitHubService;
+    }
+
+    /// <summary>
+    /// See the class comment.
+    /// </summary>
+    public async Task<List<string>> GetFromBuildDefinitionRepoCodeowners(BuildDefinition buildDefinition)
+    {
+        if (buildDefinition.Process.Type != BuildDefinitionYamlProcessType)
+        {
+            this.log.LogDebug(
+                "buildDefinition.Process.Type: '{buildDefinitionProcessType}' " + 
+                "for buildDefinition.Name: '{buildDefinitionName}' " +
+                "must be '{BuildDefinitionYamlProcessType}'.",
+                buildDefinition.Process.Type,
+                buildDefinition.Name,
+                BuildDefinitionYamlProcessType);
+            return null;
+        }
+        YamlProcess yamlProcess = (YamlProcess)buildDefinition.Process;
+
+        Uri repoUrl = GetCodeownersRepoUrl(buildDefinition);
+        if (repoUrl == null)
+        {
+            // assert: the reason why repoUrl is null has been already logged.
+            return null;
+        }
+
+        List<CodeownersEntry> codeownersEntries = await gitHubService.GetCodeownersFileEntries(repoUrl);
+        if (codeownersEntries == null)
+        {
+            this.log.LogInformation("CODEOWNERS file in '{repoUrl}' not found. Skipping sync.", repoUrl);
+            return null;
+        }
+
+        // yamlProcess.YamlFilename is misleading here. It is actually a file path, not file name.
+        // E.g. it is "sdk/foo_service/ci.yml".
+        string buildDefinitionFilePath = yamlProcess.YamlFilename; 
+
+        this.log.LogInformation(
+            "Searching CODEOWNERS for matching path for '{buildDefinitionFilePath}'",
+            buildDefinitionFilePath);
+
+        CodeownersEntry matchingCodeownersEntry = GetMatchingCodeownersEntry(yamlProcess, codeownersEntries);
+        List<string> contacts = matchingCodeownersEntry.Owners;
+
+        this.log.LogInformation(
+            "Found matching contacts (owners) in CODEOWNERS. " +
+            "Searched path '{buildDefinitionOwnerFile}', Contacts#: {contactsCount}",
+            buildDefinitionFilePath,
+            contacts.Count);
+
+        return contacts;
+    }
+
+    private Uri GetCodeownersRepoUrl(BuildDefinition buildDefinition)
+    {
+        Uri repoUrl = buildDefinition.Repository.Url;
+        this.log.LogInformation("Fetching CODEOWNERS file from repoUrl: '{repoUrl}'", repoUrl);
+
+        if (!string.IsNullOrEmpty(repoUrl?.ToString()))
+        {
+            repoUrl = new Uri(Regex.Replace(repoUrl.ToString(), @"\.git$", String.Empty));
+        }
+        else
+        {
+            this.log.LogError(
+                "No repository url returned from buildDefinition. " +
+                "buildDefinition.Name: '{buildDefinitionName}' " +
+                "buildDefinition.Repository.Id: {buildDefinitionRepositoryId}",
+                buildDefinition.Name,
+                buildDefinition.Repository.Id);
+        }
+
+        return repoUrl;
+    }
+
+
+    private CodeownersEntry GetMatchingCodeownersEntry(YamlProcess process, List<CodeownersEntry> codeownersEntries)
+    {
+        CodeownersEntry matchingCodeownersEntry =
+            CodeownersFile.GetMatchingCodeownersEntry(process.YamlFilename, codeownersEntries);
+
+        matchingCodeownersEntry.ExcludeNonUserAliases();
+
+        return matchingCodeownersEntry;
+    }
+}

tools/notification-configuration/notification-creator/NotificationConfigurator.cs

@@ -12,8 +12,6 @@
 using System.Threading.Tasks;
 using Azure.Sdk.Tools.NotificationConfiguration.Helpers;
 using System;
-using Azure.Sdk.Tools.CodeOwnersParser;
-using System.Text.RegularExpressions;
 
 namespace Azure.Sdk.Tools.NotificationConfiguration
 {
@@ -24,11 +22,10 @@ class NotificationConfigurator
         private readonly ILogger<NotificationConfigurator> logger;
 
         private const int MaxTeamNameLength = 64;
-        // Type 2 maps to a pipeline YAML file in the repository
-        private const int PipelineYamlProcessType = 2;
+
         // A cache on the code owners github identity to owner descriptor.
-        private readonly Dictionary<string, string> codeOwnerCache = new Dictionary<string, string>();
-        // A cache on the team member to member discriptor.
+        private readonly Dictionary<string, string> contactsCache = new Dictionary<string, string>();
+        // A cache on the team member to member descriptor.
         private readonly Dictionary<string, string> teamMemberCache = new Dictionary<string, string>();
 
         public NotificationConfigurator(AzureDevOpsService service, GitHubService gitHubService, ILogger<NotificationConfigurator> logger)
@@ -171,72 +168,51 @@ private async Task<WebApiTeam> EnsureTeamExists(
 
             if (purpose == TeamPurpose.SynchronizedNotificationTeam)
             {
-                await SyncTeamWithCodeOwnerFile(pipeline, result, gitHubToAADConverter, gitHubService, persistChanges);
+                await SyncTeamWithCodeownersFile(pipeline, result, gitHubToAADConverter, persistChanges);
             }
             return result;
         }
 
-        private async Task SyncTeamWithCodeOwnerFile(BuildDefinition pipeline, WebApiTeam team, GitHubToAADConverter gitHubToAADConverter, GitHubService gitHubService, bool persistChanges)
+        private async Task SyncTeamWithCodeownersFile(
+            BuildDefinition buildDefinition,
+            WebApiTeam team,
+            GitHubToAADConverter gitHubToAADConverter,
+            bool persistChanges)
         {
             using (logger.BeginScope("Team Name = {0}", team.Name))
             {
-                if (pipeline.Process.Type != PipelineYamlProcessType)
-                {
-                    return;
-                }
-
-                // Get contents of CODEOWNERS
-                Uri repoUrl = pipeline.Repository.Url;
-                logger.LogInformation("Fetching CODEOWNERS file from repo url '{repoUrl}'", repoUrl);
-
-                if (repoUrl != null)
-                {
-                    repoUrl = new Uri(Regex.Replace(repoUrl.ToString(), @"\.git$", String.Empty));
-                }
-                else
-                {
-                    logger.LogError("No repository url returned from pipeline. Repo id: {0}", pipeline.Repository.Id);
-                    return;
-                }
-                var codeOwnerEntries = await gitHubService.GetCodeownersFile(repoUrl);
-
-                if (codeOwnerEntries == default)
+                List<string> contacts =
+                    await new Contacts(gitHubService, logger).GetFromBuildDefinitionRepoCodeowners(buildDefinition);
+                if (contacts == null)
                 {
-                    logger.LogInformation("CODEOWNERS file not found, skipping sync");
+                    // assert: the reason for why contacts is null has been already logged.
                     return;
                 }
-                var process = pipeline.Process as YamlProcess;
-
-                logger.LogInformation("Searching CODEOWNERS for matching path for {0}", process.YamlFilename);
-
-                var codeOwnerEntry = CodeownersFile.GetMatchingCodeownersEntry(process.YamlFilename, codeOwnerEntries);
-                codeOwnerEntry.ExcludeNonUserAliases();
-
-                logger.LogInformation("Matching Contacts Path = {0}, NumContacts = {1}", process.YamlFilename, codeOwnerEntry.Owners.Count);
 
                 // Get set of team members in the CODEOWNERS file
-                var codeownersDescriptors = new List<String>();
-                foreach (var contact in codeOwnerEntry.Owners)
+                var contactsDescriptors = new List<string>();
+                foreach (string contact in contacts)
                 {
-                    if (!codeOwnerCache.ContainsKey(contact))
+                    if (!contactsCache.ContainsKey(contact))
                     {
                         // TODO: Better to have retry if no success on this call.
                         var userPrincipal = gitHubToAADConverter.GetUserPrincipalNameFromGithub(contact);
                         if (!string.IsNullOrEmpty(userPrincipal))
                         {
-                            codeOwnerCache[contact] = await service.GetDescriptorForPrincipal(userPrincipal);
+                            contactsCache[contact] = await service.GetDescriptorForPrincipal(userPrincipal);
                         }
                         else
                         {
-                            logger.LogInformation("Cannot find the user principal for github {0}", contact);
-                            codeOwnerCache[contact] = null;
+                            logger.LogInformation(
+                                "Cannot find the user principal for GitHub contact '{contact}'",
+                                contact);
+                            contactsCache[contact] = null;
                         }
                     }
-                    codeownersDescriptors.Add(codeOwnerCache[contact]);
+                    contactsDescriptors.Add(contactsCache[contact]);
                 }
 
-
-                var codeownersSet = new HashSet<string>(codeownersDescriptors);
+                var contactsSet = new HashSet<string>(contactsDescriptors);
                 // Get set of team members in the DevOps teams
                 var teamMembers = await service.GetMembersAsync(team);
                 var teamDescriptors = new List<String>();
@@ -250,24 +226,24 @@ private async Task SyncTeamWithCodeOwnerFile(BuildDefinition pipeline, WebApiTea
                     teamDescriptors.Add(teamMemberCache[member.Identity.Id]);
                 }
                 var teamSet = new HashSet<string>(teamDescriptors);
-                var contactsToRemove = teamSet.Except(codeownersSet);
-                var contactsToAdd = codeownersSet.Except(teamSet);
+                var contactsToRemove = teamSet.Except(contactsSet);
+                var contactsToAdd = contactsSet.Except(teamSet);
 
-                foreach (var descriptor in contactsToRemove)
+                foreach (string descriptor in contactsToRemove)
                 {
                     if (persistChanges && descriptor != null)
                     {
-                        var teamDescriptor = await service.GetDescriptorAsync(team.Id);
+                        string teamDescriptor = await service.GetDescriptorAsync(team.Id);
                         logger.LogInformation("Delete Contact TeamDescriptor = {0}, ContactDescriptor = {1}", teamDescriptor, descriptor);
                         await service.RemoveMember(teamDescriptor, descriptor);
                     }
                 }
 
-                foreach (var descriptor in contactsToAdd)
+                foreach (string descriptor in contactsToAdd)
                 {
                     if (persistChanges && descriptor != null)
                     {
-                        var teamDescriptor = await service.GetDescriptorAsync(team.Id);
+                        string teamDescriptor = await service.GetDescriptorAsync(team.Id);
                         logger.LogInformation("Add Contact TeamDescriptor = {0}, ContactDescriptor = {1}", teamDescriptor, descriptor);
                         await service.AddToTeamAsync(teamDescriptor, descriptor);
                     }