Skip to content
Draft
44 changes: 44 additions & 0 deletions tools/identity-resolution/Services/AzureDevOpsService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,50 @@ public async Task<List<TeamMember>> GetMembersAsync(WebApiTeam team)
return members;
}

/// <summary>
/// Gets descriptors for all team members (batch operation)
/// </summary>
/// <param name="team">Team</param>
/// <returns>List of member descriptors, or null if an error occurs</returns>
public async Task<IEnumerable<string>> GetMemberDescriptorsAsync(WebApiTeam team)
{
var client = await GetClientAsync<TeamHttpClient>();

logger.LogInformation("GetMemberDescriptorsAsync TeamId = {0}, TeamName = {1}", team.Id, team.Name);

try
{
var members = await client.GetTeamMembersWithExtendedPropertiesAsync(
team.ProjectId.ToString(),
team.Id.ToString()
);

var descriptors = new List<string>();
var identityClient = await GetClientAsync<IdentityHttpClient>();

foreach (var member in members)
{
try
{
var identity = await identityClient.ReadIdentityAsync(new Guid(member.Identity.Id));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot why is this more efficient the original GetUserFromId? It seems like we are still making an api call per member to get the descriptor. If so please revert this and go back to what were doing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're correct - GetMemberDescriptorsAsync provided no optimization as it still made one ReadIdentityAsync call per member. Reverted to the original GetUserFromId approach with teamMemberCache in 1719bc1.

descriptors.Add(identity.SubjectDescriptor.ToString());
}
catch (Exception ex)
{
logger.LogWarning(ex, "Failed to get descriptor for team member {memberId}", member.Identity.Id);
// Continue with other members
}
}

return descriptors;
}
catch (Exception ex)
{
logger.LogWarning(ex, "Failed to get member descriptors for team {teamId}, falling back to individual calls", team.Id);
return null; // Return null to indicate error and trigger fallback
}
}

/// <summary>
///
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ class NotificationConfigurator

// A cache on the code owners github identity to owner descriptor.
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)
{
Expand Down Expand Up @@ -121,70 +119,112 @@ private async Task SyncTeamWithCodeownersFile(
return;
}

// Get set of team members in the CODEOWNERS file
var contactsDescriptors = new List<string>();
foreach (string contact in contacts)
var distinctContacts = contacts
.Where(contact => !string.IsNullOrEmpty(contact))
.Distinct()
.ToList();

var contactsDescriptors = await ResolveContactDescriptorsAsync(distinctContacts, gitHubToAADConverter);

var contactsSet = new HashSet<string>(contactsDescriptors.Where(d => d != null));

var teamDescriptors = await GetTeamMemberDescriptorsAsync(team);

var teamSet = new HashSet<string>(teamDescriptors.Where(d => d != null));

if (contactsSet.SetEquals(teamSet))
{
if (!contactsCache.ContainsKey(contact))
logger.LogInformation("Team membership is already synchronized. No changes needed.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
logger.LogInformation("Team membership is already synchronized. No changes needed.");

return;
}

var contactsToRemove = teamSet.Except(contactsSet).ToList();
var contactsToAdd = contactsSet.Except(teamSet).ToList();

string teamDescriptor = "";
if (contactsToRemove.Any() || contactsToAdd.Any())
{
teamDescriptor = await service.GetDescriptorAsync(team.Id);

if (contactsToRemove.Any())
{
logger.LogInformation("Removing {count} contact(s) from team", contactsToRemove.Count);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (contactsToRemove.Any())
{
logger.LogInformation("Removing {count} contact(s) from team", contactsToRemove.Count);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot remove this extra logging you added similarly for the add code path

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the aggregate logging for both remove and add operations, and the team sync status message. Changes in 9f9b142.


foreach (string descriptor in contactsToRemove)
{
// TODO: Better to have retry if no success on this call.
var userPrincipal = await gitHubToAADConverter.GetUserPrincipalNameFromGithubAsync(contact);
if (!string.IsNullOrEmpty(userPrincipal))
if (persistChanges)
{
contactsCache[contact] = await service.GetDescriptorForPrincipal(userPrincipal);
logger.LogInformation("Delete Contact TeamDescriptor = {0}, ContactDescriptor = {1}", teamDescriptor, descriptor);
await service.RemoveMember(teamDescriptor, descriptor);
}
else
{
logger.LogInformation(
"Cannot find the user principal for GitHub contact '{contact}'",
contact);
contactsCache[contact] = null;
logger.LogInformation("Would delete Contact TeamDescriptor = {0}, ContactDescriptor = {1}", teamDescriptor, descriptor);
}
}
contactsDescriptors.Add(contactsCache[contact]);
}

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>();
foreach (var member in teamMembers)
{
if (!teamMemberCache.ContainsKey(member.Identity.Id))
if (contactsToAdd.Any())
{
var teamMemberDescriptor = (await service.GetUserFromId(new Guid(member.Identity.Id))).SubjectDescriptor.ToString();
teamMemberCache[member.Identity.Id] = teamMemberDescriptor;
logger.LogInformation("Adding {count} contact(s) to team", contactsToAdd.Count);
}
teamDescriptors.Add(teamMemberCache[member.Identity.Id]);
}
var teamSet = new HashSet<string>(teamDescriptors);
var contactsToRemove = teamSet.Except(contactsSet);
var contactsToAdd = contactsSet.Except(teamSet);
string teamDescriptor = "";

if (contactsToRemove.Any() || contactsToAdd.Any())
{
teamDescriptor = await service.GetDescriptorAsync(team.Id);
}

foreach (string descriptor in contactsToRemove)
{
if (persistChanges && descriptor != null)

foreach (string descriptor in contactsToAdd)
{
logger.LogInformation("Delete Contact TeamDescriptor = {0}, ContactDescriptor = {1}", teamDescriptor, descriptor);
await service.RemoveMember(teamDescriptor, descriptor);
if (persistChanges)
{
logger.LogInformation("Add Contact TeamDescriptor = {0}, ContactDescriptor = {1}", teamDescriptor, descriptor);
await service.AddToTeamAsync(teamDescriptor, descriptor);
}
else
{
logger.LogInformation("Would add Contact TeamDescriptor = {0}, ContactDescriptor = {1}", teamDescriptor, descriptor);
}
}
}
}
}

foreach (string descriptor in contactsToAdd)
/// <summary>
/// Resolves contact GitHub handles to descriptors with caching
/// </summary>
private async Task<List<string>> ResolveContactDescriptorsAsync(
List<string> contacts,
GitHubToAADConverter gitHubToAADConverter)
{
var contactsDescriptors = new List<string>();

foreach (string contact in contacts)
{
if (!contactsCache.ContainsKey(contact))
{
if (persistChanges && descriptor != null)
var userPrincipal = await gitHubToAADConverter.GetUserPrincipalNameFromGithubAsync(contact);
if (!string.IsNullOrEmpty(userPrincipal))
{
contactsCache[contact] = await service.GetDescriptorForPrincipal(userPrincipal);
}
else
{
logger.LogInformation("Add Contact TeamDescriptor = {0}, ContactDescriptor = {1}", teamDescriptor, descriptor);
await service.AddToTeamAsync(teamDescriptor, descriptor);
logger.LogInformation(
"Cannot find the user principal for GitHub contact '{contact}'",
contact);
// Cache null results to avoid re-requesting
contactsCache[contact] = null;
}
}
contactsDescriptors.Add(contactsCache[contact]);
}

return contactsDescriptors;
}

/// <summary>
/// Gets team member descriptors
/// </summary>
private async Task<List<string>> GetTeamMemberDescriptorsAsync(WebApiTeam team)
{
var batchDescriptors = await service.GetMemberDescriptorsAsync(team);
return batchDescriptors?.ToList() ?? new List<string>();
}

private async Task<IEnumerable<BuildDefinition>> GetPipelinesAsync(string projectName, string projectPath, PipelineSelectionStrategy strategy)
Expand Down Expand Up @@ -242,6 +282,7 @@ private async Task EnsureScheduledBuildFailSubscriptionExists(BuildDefinition pi
};

logger.LogInformation("Creating Subscription PipelineId = {0}, TeamId = {1}", pipeline.Id, team.Id);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

if (persistChanges)
{
subscription = await service.CreateSubscriptionAsync(newSubscription);
Expand Down