diff --git a/tools/code-owners-parser/Azure.Sdk.Tools.CodeOwnersParser.Tests/CodeOwnersFileTests.cs b/tools/code-owners-parser/Azure.Sdk.Tools.CodeOwnersParser.Tests/CodeownersFileTests.cs similarity index 89% rename from tools/code-owners-parser/Azure.Sdk.Tools.CodeOwnersParser.Tests/CodeOwnersFileTests.cs rename to tools/code-owners-parser/Azure.Sdk.Tools.CodeOwnersParser.Tests/CodeownersFileTests.cs index 53e83978ffc..7259bed77fa 100644 --- a/tools/code-owners-parser/Azure.Sdk.Tools.CodeOwnersParser.Tests/CodeOwnersFileTests.cs +++ b/tools/code-owners-parser/Azure.Sdk.Tools.CodeOwnersParser.Tests/CodeownersFileTests.cs @@ -4,13 +4,13 @@ namespace Azure.Sdk.Tools.CodeOwnersParser.Tests; [TestFixture] -public class CodeOwnersFileTests +public class CodeownersFileTests { /// /// A battery of test cases specifying behavior of new logic matching target /// path to CODEOWNERS entries, and comparing it to existing, legacy logic. /// - /// The logic that has changed is located in CodeOwnersFile.FindOwnersForClosestMatch. + /// The logic that has changed is located in CodeownersFile.GetMatchingCodeownersEntry. /// /// In the test case table below, any discrepancy between legacy and new /// matcher expected matches that doesn't pertain to wildcard matching denotes @@ -143,33 +143,31 @@ public class CodeOwnersFileTests }; /// - /// Exercises Azure.Sdk.Tools.CodeOwnersParser.Tests.CodeOwnersFileTests.testCases. + /// Exercises Azure.Sdk.Tools.CodeOwnersParser.Tests.CodeownersFileTests.testCases. /// See comment on that member for details. /// [TestCaseSource(nameof(testCases))] - public void TestParseAndFindOwnersForClosestMatch(TestCase testCase) + public void TestGetMatchingCodeownersEntry(TestCase testCase) { - List? codeownersEntries = - CodeOwnersFile.ParseContent(testCase.CodeownersPath + "@owner"); + List? codeownersEntries = + CodeownersFile.GetCodeownersEntries(testCase.CodeownersPath + "@owner"); - VerifyFindOwnersForClosestMatch(testCase, codeownersEntries, useNewImpl: false, testCase.ExpectedLegacyMatch); - VerifyFindOwnersForClosestMatch(testCase, codeownersEntries, useNewImpl: true, testCase.ExpectedNewMatch); + VerifyGetMatchingCodeownersEntry(testCase, codeownersEntries, useNewImpl: false, testCase.ExpectedLegacyMatch); + VerifyGetMatchingCodeownersEntry(testCase, codeownersEntries, useNewImpl: true, testCase.ExpectedNewMatch); } - private static void VerifyFindOwnersForClosestMatch( + private static void VerifyGetMatchingCodeownersEntry( TestCase testCase, - List codeownersEntries, + List codeownersEntries, bool useNewImpl, bool expectedMatch) { - CodeOwnerEntry? entryLegacy = + CodeownersEntry? entryLegacy = // Act - CodeOwnersFile.FindOwnersForClosestMatch( - codeownersEntries, - testCase.TargetPath, - useNewFindOwnersForClosestMatchImpl: useNewImpl); + CodeownersFile.GetMatchingCodeownersEntry(testCase.TargetPath, + codeownersEntries, useNewImpl: useNewImpl); - Assert.That(entryLegacy.Owners.Count, Is.EqualTo(expectedMatch ? 1 : 0)); + Assert.That(entryLegacy.Owners, Has.Count.EqualTo(expectedMatch ? 1 : 0)); } public record TestCase( diff --git a/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/ConsoleOutput.cs b/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/ConsoleOutput.cs index a4de98d7690..434129f87c4 100644 --- a/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/ConsoleOutput.cs +++ b/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/ConsoleOutput.cs @@ -38,6 +38,8 @@ public void Dispose() Console.SetOut(this.originalOutput); this.stringWriter.Dispose(); this.originalOutput.Dispose(); + // https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1816 + GC.SuppressFinalize(this); } /// diff --git a/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/MainTests.cs b/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/MainTests.cs deleted file mode 100644 index 51701637e91..00000000000 --- a/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/MainTests.cs +++ /dev/null @@ -1,58 +0,0 @@ -using System.Collections.Generic; -using System.Text.Json; -using Azure.Sdk.Tools.CodeOwnersParser; -using NUnit.Framework; - -namespace Azure.Sdk.Tools.RetrieveCodeOwners.Tests -{ - /// - /// This is test of console app Azure.Sdk.Tools.RetrieveCodeOwners. - /// - [TestFixture] - public class MainTests - { - private const string CodeOwnersFilePath = "CODEOWNERS"; - - private static readonly object[] sourceLists = - { - new object[] {"sdk", false, new List { "person1", "person2" } }, - new object[] { "/sdk", false, new List { "person1", "person2" } }, - new object[] { "sdk/noPath", false, new List { "person1", "person2" } }, - new object[] { "/sdk/azconfig", false, new List { "person3", "person4" } }, - new object[] { "/sdk/azconfig/package", false, new List { "person3", "person4" } }, - new object[] { "/sdk/testUser/", true, new List { "azure-sdk" } }, - new object[] { "/sd", true, new List() } - }; - - [TestCaseSource(nameof(sourceLists))] - public void TestOnNormalOutput(string targetDirectory, bool includeUserAliasesOnly, List expectedReturn) - { - using (var consoleOutput = new ConsoleOutput()) - { - Program.Main(CodeOwnersFilePath, targetDirectory, includeUserAliasesOnly); - var output = consoleOutput.GetOutput(); - TestExpectResult(expectedReturn, output); - consoleOutput.Dispose(); - } - } - - [TestCase("PathNotExist")] - [TestCase("http://testLink")] - [TestCase("https://testLink")] - public void TestOnError(string codeOwnerPath) - { - Assert.That(Program.Main(codeOwnerPath, "sdk"), Is.EqualTo(1)); - } - - private static void TestExpectResult(List expectReturn, string output) - { - CodeOwnerEntry? codeOwnerEntry = JsonSerializer.Deserialize(output); - List actualReturn = codeOwnerEntry!.Owners; - Assert.That(actualReturn.Count, Is.EqualTo(expectReturn.Count)); - for (int i = 0; i < actualReturn.Count; i++) - { - Assert.That(actualReturn[i], Is.EqualTo(expectReturn[i])); - } - } - } -} diff --git a/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/ProgramTests.cs b/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/ProgramTests.cs new file mode 100644 index 00000000000..87fe0a32d5d --- /dev/null +++ b/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/ProgramTests.cs @@ -0,0 +1,58 @@ +using System.Collections.Generic; +using System.Text.Json; +using Azure.Sdk.Tools.CodeOwnersParser; +using NUnit.Framework; + +namespace Azure.Sdk.Tools.RetrieveCodeOwners.Tests +{ + /// + /// Test class for Azure.Sdk.Tools.RetrieveCodeOwners.Program. + /// + [TestFixture] + public class ProgramTests + { + private const string CodeownersFilePath = "CODEOWNERS"; + + private static readonly object[] sourceLists = + { + new object[] {"sdk", false, new List { "person1", "person2" } }, + new object[] { "/sdk", false, new List { "person1", "person2" } }, + new object[] { "sdk/noPath", false, new List { "person1", "person2" } }, + new object[] { "/sdk/azconfig", false, new List { "person3", "person4" } }, + new object[] { "/sdk/azconfig/package", false, new List { "person3", "person4" } }, + new object[] { "/sdk/testUser/", true, new List { "azure-sdk" } }, + new object[] { "/sd", true, new List() } + }; + + [TestCaseSource(nameof(sourceLists))] + public void TestOnNormalOutput(string targetPath, bool excludeNonUserAliases, List expectedOwners) + { + using var consoleOutput = new ConsoleOutput(); + + // Act + Program.Main(targetPath, CodeownersFilePath, excludeNonUserAliases); + + string actualOutput = consoleOutput.GetOutput(); + AssertOwners(actualOutput, expectedOwners); + } + + [TestCase("PathNotExist")] + [TestCase("http://testLink")] + [TestCase("https://testLink")] + public void TestOnError(string codeownersPath) + { + Assert.That(Program.Main("sdk", codeownersPath), Is.EqualTo(1)); + } + + private static void AssertOwners(string actualOutput, List expectedOwners) + { + CodeownersEntry? actualEntry = JsonSerializer.Deserialize(actualOutput); + List actualOwners = actualEntry!.Owners; + Assert.That(actualOwners, Has.Count.EqualTo(expectedOwners.Count)); + for (int i = 0; i < actualOwners.Count; i++) + { + Assert.That(actualOwners[i], Is.EqualTo(expectedOwners[i])); + } + } + } +} diff --git a/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners/Program.cs b/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners/Program.cs index efc105654b7..10f8df3486f 100644 --- a/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners/Program.cs +++ b/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners/Program.cs @@ -5,36 +5,47 @@ namespace Azure.Sdk.Tools.RetrieveCodeOwners { /// - /// The tool command to retrieve code owners. + /// See Program.Main comment. /// public static class Program { /// - /// Retrieves CODEOWNERS information for specific section of the repo + /// Given targetPath and CODEOWNERS file path or https url codeownersFilePathOrUrl, + /// prints out to stdout owners of the targetPath as determined by the CODEOWNERS data. /// - /// The path of CODEOWNERS file in repo - /// The directory whose information is to be retrieved - /// The option to filter out code owner team alias. - /// Exit code - + /// The path whose owners are to be determined. + /// The https url or path to the CODEOWNERS file. + /// Whether owners that aren't users should be excluded from the + /// returned owners. + /// + /// On STDOUT: The JSON representation of the matched CodeownersEntry. + /// "new CodeownersEntry()" if no path in the CODEOWNERS data matches. + ///

+ /// From the Main method: exit code. 0 if successful, 1 if error. + /// public static int Main( - string codeOwnerFilePath, - string targetDirectory, - bool filterOutNonUserAliases = false - ) + string targetPath, + string codeownersFilePathOrUrl, + bool excludeNonUserAliases = false) { - var target = targetDirectory.Trim(); - try { - var codeOwnerEntry = CodeOwnersFile.ParseAndFindOwnersForClosestMatch(codeOwnerFilePath, target); - if (filterOutNonUserAliases) + targetPath = targetPath.Trim(); + try + { + var codeownersEntry = CodeownersFile.GetMatchingCodeownersEntry(targetPath, codeownersFilePathOrUrl); + if (excludeNonUserAliases) { - codeOwnerEntry.FilterOutNonUserAliases(); + codeownersEntry.ExcludeNonUserAliases(); } - var codeOwnerJson = JsonSerializer.Serialize(codeOwnerEntry, new JsonSerializerOptions { WriteIndented = true }); - Console.WriteLine(codeOwnerJson); + + var codeownersJson = JsonSerializer.Serialize( + codeownersEntry, + new JsonSerializerOptions { WriteIndented = true }); + + Console.WriteLine(codeownersJson); return 0; } - catch (Exception e) { + catch (Exception e) + { Console.Error.WriteLine(e.Message); return 1; } diff --git a/tools/code-owners-parser/CodeOwnersParser/CodeOwnersFile.cs b/tools/code-owners-parser/CodeOwnersParser/CodeOwnersFile.cs deleted file mode 100644 index 0e7d4285973..00000000000 --- a/tools/code-owners-parser/CodeOwnersParser/CodeOwnersFile.cs +++ /dev/null @@ -1,109 +0,0 @@ -using System; -using System.Collections.Generic; -using System.IO; - -namespace Azure.Sdk.Tools.CodeOwnersParser -{ - public static class CodeOwnersFile - { - public static List ParseFile(string filePathOrUrl) - { - string content = FileHelpers.GetFileContents(filePathOrUrl); - return ParseContent(content); - } - - public static List ParseContent(string fileContent) - { - List entries = new List(); - - // An entry ends when we get to a path (a real path or a commented dummy path) - using (StringReader sr = new StringReader(fileContent)) - { - CodeOwnerEntry entry = new CodeOwnerEntry(); - - // we are going to read line by line until we find a line that is not a comment OR that is using the placeholder entry inside the comment. - // while we are trying to find the folder entry, we parse all comment lines to extract the labels from it. - // when we find the path or placeholder, we add the completed entry and create a new one. - while (sr.ReadLine() is { } line) - { - line = NormalizeLine(line); - - if (string.IsNullOrWhiteSpace(line)) - { - continue; - } - - if (!line.StartsWith("#") || line.IndexOf(CodeOwnerEntry.MissingFolder, System.StringComparison.OrdinalIgnoreCase) >= 0) - { - // If this is not a comment line OR this is a placeholder entry - - entry.ParseOwnersAndPath(line); - - // only add it if it is a valid entry - if (entry.IsValid) - { - entries.Add(entry); - } - - // create a new entry. - entry = new CodeOwnerEntry(); - } - else if (line.StartsWith("#")) - { - // try to process the line in case there are markers that need to be extracted - entry.ProcessLabelsOnLine(line); - } - - } - } - return entries; - } - - public static CodeOwnerEntry ParseAndFindOwnersForClosestMatch( - string codeOwnersFilePathOrUrl, - string targetPath, - bool useNewFindOwnersForClosestMatchImpl = false) - { - var codeOwnerEntries = ParseFile(codeOwnersFilePathOrUrl); - return FindOwnersForClosestMatch(codeOwnerEntries, targetPath, useNewFindOwnersForClosestMatchImpl); - } - - public static CodeOwnerEntry FindOwnersForClosestMatch( - List codeOwnerEntries, - string targetPath, - bool useNewFindOwnersForClosestMatchImpl = false) - { - return useNewFindOwnersForClosestMatchImpl - ? new MatchedCodeOwnerEntry(codeOwnerEntries, targetPath).Value - : FindOwnersForClosestMatchLegacyImpl(codeOwnerEntries, targetPath); - } - - private static CodeOwnerEntry FindOwnersForClosestMatchLegacyImpl(List codeOwnerEntries, - string targetPath) - { - // Normalize the start and end of the paths by trimming slash - targetPath = targetPath.Trim('/'); - - // We want to find the match closest to the bottom of the codeowners file. - // CODEOWNERS sorts the paths in order of 'RepoPath', 'ServicePath' and then 'PackagePath'. - for (int i = codeOwnerEntries.Count - 1; i >= 0; i--) - { - string codeOwnerPath = codeOwnerEntries[i].PathExpression.Trim('/'); - // Note that this only matches on paths without glob patterns which is good enough - // for our current scenarios but in the future might need to support globs - if (targetPath.StartsWith(codeOwnerPath, StringComparison.OrdinalIgnoreCase)) - { - return codeOwnerEntries[i]; - } - } - - return new CodeOwnerEntry(); - } - - private static string NormalizeLine(string line) - => !string.IsNullOrEmpty(line) - // Remove tabs and trim extra whitespace - ? line.Replace('\t', ' ').Trim() - : line; - } -} diff --git a/tools/code-owners-parser/CodeOwnersParser/CodeOwnerEntry.cs b/tools/code-owners-parser/CodeOwnersParser/CodeownersEntry.cs similarity index 67% rename from tools/code-owners-parser/CodeOwnersParser/CodeOwnerEntry.cs rename to tools/code-owners-parser/CodeOwnersParser/CodeownersEntry.cs index c2fa426734a..ce5558c9882 100644 --- a/tools/code-owners-parser/CodeOwnersParser/CodeOwnerEntry.cs +++ b/tools/code-owners-parser/CodeOwnersParser/CodeownersEntry.cs @@ -6,11 +6,14 @@ namespace Azure.Sdk.Tools.CodeOwnersParser { /// /// The entry for CODEOWNERS has the following structure: - /// # PRLabel: %Label - /// # ServiceLabel: %Label - /// path @owner @owner + /// + /// + /// # PRLabel: %Label + /// # ServiceLabel: %Label + /// path @owner @owner + /// /// - public class CodeOwnerEntry + public class CodeownersEntry { const char LabelSeparator = '%'; const char OwnerSeparator = '@'; @@ -20,7 +23,7 @@ public class CodeOwnerEntry public string PathExpression { get; set; } = ""; - public bool ContainsWildcard => PathExpression.Contains("*"); + public bool ContainsWildcard => PathExpression.Contains('*'); public List Owners { get; set; } = new List(); @@ -28,32 +31,24 @@ public class CodeOwnerEntry public List ServiceLabels { get; set; } = new List(); - public bool IsValid - { - get - { - return !string.IsNullOrEmpty(PathExpression); - } - } + public bool IsValid => !string.IsNullOrWhiteSpace(PathExpression); private static string[] SplitLine(string line, char splitOn) - { - return line.Split(new char[] { splitOn }, StringSplitOptions.RemoveEmptyEntries); - } + => line.Split(new char[] { splitOn }, StringSplitOptions.RemoveEmptyEntries); public override string ToString() - { - return $"HasWildcard:{ContainsWildcard} Expression:{PathExpression} Owners:{string.Join(",", Owners)} PRLabels:{string.Join(",", PRLabels)} ServiceLabels:{string.Join(",", ServiceLabels)}"; - } + => $"HasWildcard:{ContainsWildcard} Expression:{PathExpression} " + + $"Owners:{string.Join(",", Owners)} PRLabels:{string.Join(",", PRLabels)} " + + $"ServiceLabels:{string.Join(",", ServiceLabels)}"; public bool ProcessLabelsOnLine(string line) { - if (line.IndexOf(PRLabelMoniker, StringComparison.OrdinalIgnoreCase) >= 0) + if (line.Contains(PRLabelMoniker, StringComparison.OrdinalIgnoreCase)) { PRLabels.AddRange(ParseLabels(line, PRLabelMoniker)); return true; } - else if (line.IndexOf(ServiceLabelMoniker, StringComparison.OrdinalIgnoreCase) >= 0) + else if (line.Contains(ServiceLabelMoniker, StringComparison.OrdinalIgnoreCase)) { ServiceLabels.AddRange(ParseLabels(line, ServiceLabelMoniker)); return true; @@ -61,10 +56,10 @@ public bool ProcessLabelsOnLine(string line) return false; } - private IEnumerable ParseLabels(string line, string moniker) + private static IEnumerable ParseLabels(string line, string moniker) { // Parse a line that looks like # PRLabel: %Label, %Label - if (line.IndexOf(moniker, StringComparison.OrdinalIgnoreCase) == -1) + if (!line.Contains(moniker, StringComparison.OrdinalIgnoreCase)) { yield break; } @@ -76,7 +71,7 @@ private IEnumerable ParseLabels(string line, string moniker) yield break; } - line = line.Substring(colonPosition + 1).Trim(); + line = line[(colonPosition + 1)..].Trim(); foreach (string label in SplitLine(line, LabelSeparator).ToList()) { if (!string.IsNullOrWhiteSpace(label)) @@ -88,32 +83,40 @@ private IEnumerable ParseLabels(string line, string moniker) public void ParseOwnersAndPath(string line) { - if (string.IsNullOrEmpty(line) || - (line.StartsWith("#") && !(line.IndexOf(CodeOwnerEntry.MissingFolder, StringComparison.OrdinalIgnoreCase) >= 0))) + if ( + string.IsNullOrEmpty(line) + || (IsComment(line) + && !line.Contains( + CodeownersEntry.MissingFolder, StringComparison.OrdinalIgnoreCase))) { return; } line = ParsePath(line); - - //remove any comments from the line, if any. - // this is the case when we have something like @user #comment - int commentIndex = line.IndexOf("#"); - - if (commentIndex >= 0) - { - line = line.Substring(0, commentIndex).Trim(); - } + line = RemoveCommentIfAny(line); foreach (string author in SplitLine(line, OwnerSeparator).ToList()) { if (!string.IsNullOrWhiteSpace(author)) - { Owners.Add(author.Trim()); - } } } + private static bool IsComment(string line) + => line.StartsWith("#"); + + private static string RemoveCommentIfAny(string line) + { + // this is the case when we have something like @user #comment + + int commentIndex = line.IndexOf("#", StringComparison.OrdinalIgnoreCase); + + if (commentIndex >= 0) + line = line[..commentIndex].Trim(); + + return line; + } + private string ParsePath(string line) { // Get the start of the owner in the string @@ -123,22 +126,19 @@ private string ParsePath(string line) return line; } - string path = line.Substring(0, ownerStartPosition).Trim(); + string path = line[..ownerStartPosition].Trim(); // the first entry is the path/regex PathExpression = path; // remove the path from the string. - return line.Substring(ownerStartPosition); + return line[ownerStartPosition..]; } /// /// Remove all code owners which are not github alias. /// - public void FilterOutNonUserAliases() - { - Owners.RemoveAll(r => !IsGitHubUserAlias(r)); - } - + public void ExcludeNonUserAliases() + => Owners.RemoveAll(r => !IsGitHubUserAlias(r)); /// /// Helper method to check if it is valid github alias. diff --git a/tools/code-owners-parser/CodeOwnersParser/CodeownersFile.cs b/tools/code-owners-parser/CodeOwnersParser/CodeownersFile.cs new file mode 100644 index 00000000000..bb77161aaae --- /dev/null +++ b/tools/code-owners-parser/CodeOwnersParser/CodeownersFile.cs @@ -0,0 +1,124 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; + +namespace Azure.Sdk.Tools.CodeOwnersParser +{ + public static class CodeownersFile + { + public static List GetCodeownersEntriesFromFileOrUrl(string codeownersFilePathOrUrl) + { + string content = FileHelpers.GetFileOrUrlContents(codeownersFilePathOrUrl); + return GetCodeownersEntries(content); + } + + public static List GetCodeownersEntries(string codeownersContent) + { + List entries = new List(); + + // We are going to read line by line until we find a line that is not a comment + // OR that is using the placeholder entry inside the comment. + // while we are trying to find the folder entry, we parse all comment lines + // to extract the labels from it. when we find the path or placeholder, + // we add the completed entry and create a new one. + CodeownersEntry entry = new CodeownersEntry(); + using StringReader sr = new StringReader(codeownersContent); + while (sr.ReadLine() is { } line) + { + entry = ProcessCodeownersLine(line, entry, entries); + } + + return entries; + } + + public static CodeownersEntry GetMatchingCodeownersEntry( + string targetPath, + string codeownersFilePathOrUrl, + bool useNewImpl = false) + { + var codeownersEntries = GetCodeownersEntriesFromFileOrUrl(codeownersFilePathOrUrl); + return GetMatchingCodeownersEntry(targetPath, codeownersEntries, useNewImpl); + } + + public static CodeownersEntry GetMatchingCodeownersEntry( + string targetPath, + List codeownersEntries, + bool useNewImpl = false) + { + Debug.Assert(targetPath != null); + return useNewImpl + ? new MatchedCodeownersEntry(targetPath, codeownersEntries).Value + : GetMatchingCodeownersEntryLegacyImpl(targetPath, codeownersEntries); + } + + private static CodeownersEntry ProcessCodeownersLine( + string line, + CodeownersEntry entry, + List entries) + { + line = NormalizeLine(line); + + if (string.IsNullOrWhiteSpace(line)) + { + return entry; + } + + if (!IsCommentLine(line) || (IsCommentLine(line) && IsPlaceholderEntry(line))) + { + entry.ParseOwnersAndPath(line); + + if (entry.IsValid) + entries.Add(entry); + + // An entry has ended, as we got to a path: real bath or placeholder path. + return new CodeownersEntry(); + } + + if (IsCommentLine(line)) + { + // try to process the line in case there are markers that need to be extracted + entry.ProcessLabelsOnLine(line); + return entry; + } + + throw new InvalidOperationException( + $"This case shouldn't be possible. line: '{line}'"); + } + + private static bool IsPlaceholderEntry(string line) + => line.Contains(CodeownersEntry.MissingFolder, StringComparison.OrdinalIgnoreCase); + + private static bool IsCommentLine(string line) + => line.StartsWith("#"); + + private static string NormalizeLine(string line) + => !string.IsNullOrEmpty(line) + // Remove tabs and trim extra whitespace + ? line.Replace('\t', ' ').Trim() + : line; + + private static CodeownersEntry GetMatchingCodeownersEntryLegacyImpl( + string targetPath, + List codeownersEntries) + { + // Normalize the start and end of the paths by trimming slash + targetPath = targetPath.Trim('/'); + + // We want to find the match closest to the bottom of the codeowners file. + // CODEOWNERS sorts the paths in order of 'RepoPath', 'ServicePath' and then 'PackagePath'. + for (int i = codeownersEntries.Count - 1; i >= 0; i--) + { + string codeownersPath = codeownersEntries[i].PathExpression.Trim('/'); + // Note that this only matches on paths without glob patterns which is good enough + // for our current scenarios but in the future might need to support globs + if (targetPath.StartsWith(codeownersPath, StringComparison.OrdinalIgnoreCase)) + { + return codeownersEntries[i]; + } + } + + return new CodeownersEntry(); + } + } +} diff --git a/tools/code-owners-parser/CodeOwnersParser/FileHelpers.cs b/tools/code-owners-parser/CodeOwnersParser/FileHelpers.cs index 2c9b0ff58e2..91db19aa3e1 100644 --- a/tools/code-owners-parser/CodeOwnersParser/FileHelpers.cs +++ b/tools/code-owners-parser/CodeOwnersParser/FileHelpers.cs @@ -6,25 +6,26 @@ namespace Azure.Sdk.Tools.CodeOwnersParser { public static class FileHelpers { - public static string GetFileContents(string fileOrUri) + public static string GetFileOrUrlContents(string fileOrUrl) { - // try to parse it as an Uri - if (fileOrUri.StartsWith("https")) - { - using (HttpClient client = new HttpClient()) - { - HttpResponseMessage response = client.GetAsync(fileOrUri).ConfigureAwait(false).GetAwaiter().GetResult(); - return response.Content.ReadAsStringAsync().ConfigureAwait(false).GetAwaiter().GetResult(); - } - } + if (fileOrUrl.StartsWith("https")) + return GetUrlContents(fileOrUrl); - string fullPath = Path.GetFullPath(fileOrUri); + string fullPath = Path.GetFullPath(fileOrUrl); if (File.Exists(fullPath)) - { return File.ReadAllText(fullPath); - } - throw new ArgumentException($"The path provided is neither local path nor https link. Please check your path: {fileOrUri} resolved to {fullPath}."); + throw new ArgumentException( + "The path provided is neither local path nor https link. " + + $"Please check your path: '{fileOrUrl}' resolved to '{fullPath}'."); + } + + private static string GetUrlContents(string url) + { + using HttpClient client = new HttpClient(); + HttpResponseMessage response = + client.GetAsync(url).ConfigureAwait(false).GetAwaiter().GetResult(); + return response.Content.ReadAsStringAsync().ConfigureAwait(false).GetAwaiter().GetResult(); } } } diff --git a/tools/code-owners-parser/CodeOwnersParser/MatchedCodeOwnerEntry.cs b/tools/code-owners-parser/CodeOwnersParser/MatchedCodeownersEntry.cs similarity index 89% rename from tools/code-owners-parser/CodeOwnersParser/MatchedCodeOwnerEntry.cs rename to tools/code-owners-parser/CodeOwnersParser/MatchedCodeownersEntry.cs index 76f3cd297ee..d828f93ea15 100644 --- a/tools/code-owners-parser/CodeOwnersParser/MatchedCodeOwnerEntry.cs +++ b/tools/code-owners-parser/CodeOwnersParser/MatchedCodeownersEntry.cs @@ -11,7 +11,7 @@ namespace Azure.Sdk.Tools.CodeOwnersParser /// /// This is a new matcher, compared to the old one, located in: /// - /// CodeOwnersFile.FindOwnersForClosestMatchLegacyImpl() + /// CodeownersFile.GetMatchingCodeownersEntryLegacyImpl() /// /// This new matcher supports matching against wildcards, while the old one doesn't. /// This new matcher is designed to work with CODEOWNERS file validation: @@ -25,7 +25,7 @@ namespace Azure.Sdk.Tools.CodeOwnersParser /// https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners#codeowners-syntax /// https://git-scm.com/docs/gitignore#_pattern_format /// - internal class MatchedCodeOwnerEntry + internal class MatchedCodeownersEntry { /// /// Token for temporarily substituting "**" in regex, to avoid it being escaped when @@ -38,7 +38,7 @@ internal class MatchedCodeOwnerEntry /// private const string SingleStar = "_SINGLE_STAR_"; - public readonly CodeOwnerEntry Value; + public readonly CodeownersEntry Value; /// /// See comment on IsCodeOwnersPathValid @@ -52,30 +52,30 @@ internal class MatchedCodeOwnerEntry /// private static readonly char[] unsupportedChars = { '[', ']', '!', '?' }; - private readonly ILogger log; + private readonly ILogger log; - public MatchedCodeOwnerEntry(List entries, string targetPath) + public MatchedCodeownersEntry(string targetPath, List codeownersEntries) { this.log = CreateLog(); - this.Value = FindOwnersForClosestMatch(entries, targetPath); + this.Value = GetMatchingCodeownersEntry(targetPath, codeownersEntries); } - private ILogger CreateLog() + private ILogger CreateLog() { var loggerFactory = LoggerFactory.Create(builder => { builder.AddSimpleConsole(); }); - return loggerFactory.CreateLogger(); + return loggerFactory.CreateLogger(); } /// - /// Returns a CodeOwnerEntry from codeOwnerEntries that matches targetPath + /// Returns a CodeownersEntry from codeOwnerEntries that matches targetPath /// per algorithm described in the GitHub CODEOWNERS reference, /// as linked to in this class comment. /// - /// If there is no match, returns "new CodeOwnerEntry()". + /// If there is no match, returns "new CodeownersEntry()". /// - private CodeOwnerEntry FindOwnersForClosestMatch( - List codeownersEntries, - string targetPath) + private CodeownersEntry GetMatchingCodeownersEntry( + string targetPath, + List codeownersEntries) { // targetPath is assumed to be absolute w.r.t. repository root, hence we ensure // it starts with "/" to denote that. @@ -87,7 +87,7 @@ private CodeOwnerEntry FindOwnersForClosestMatch( // so it can not match against a CODEOWNERS entry that is guaranteed to be a file, // by the virtue of not ending with "/". - CodeOwnerEntry matchedEntry = codeownersEntries + CodeownersEntry matchedEntry = codeownersEntries .Where(entry => !ContainsUnsupportedCharacters(entry.PathExpression)) // Entries listed in CODEOWNERS file below take precedence, hence we read the file from the bottom up. // By convention, entries in CODEOWNERS should be sorted top-down in the order of: @@ -99,7 +99,7 @@ private CodeOwnerEntry FindOwnersForClosestMatch( .FirstOrDefault( entry => Matches(targetPath, entry), // assert: none of the codeownersEntries matched targetPath - new CodeOwnerEntry()); + new CodeownersEntry()); return matchedEntry; } @@ -124,7 +124,7 @@ private bool ContainsUnsupportedCharacters(string codeownersPath) /// Returns true if the regex expression representing the PathExpression /// of CODEOWNERS entry matches a prefix of targetPath. /// - private bool Matches(string targetPath, CodeOwnerEntry entry) + private bool Matches(string targetPath, CodeownersEntry entry) { string codeownersPath = entry.PathExpression; @@ -226,7 +226,7 @@ private string NormalizePath(string codeownersPath) /// /// The entry is valid if it obeys following conditions: /// - The Value was obtained with a call to - /// Azure.Sdk.Tools.CodeOwnersParser.CodeOwnersFile.ParseContent(). + /// Azure.Sdk.Tools.CodeOwnersParser.CodeownersFile.GetCodeownersEntries(). /// - As a consequence, in the case of no match, the entry is not valid. /// - It does not contain unsupported characters (see "unsupportedChars"). /// - the Value.PathExpression starts with "/". diff --git a/tools/identity-resolution/Services/GitHubService.cs b/tools/identity-resolution/Services/GitHubService.cs index bbd40d23e3f..0e4bb73ab7d 100644 --- a/tools/identity-resolution/Services/GitHubService.cs +++ b/tools/identity-resolution/Services/GitHubService.cs @@ -16,8 +16,8 @@ public class GitHubService { private static readonly HttpClient httpClient = new HttpClient(); - private static readonly ConcurrentDictionary> - codeownersFileCache = new ConcurrentDictionary>(); + private static readonly ConcurrentDictionary> + codeownersFileCache = new ConcurrentDictionary>(); private readonly ILogger logger; @@ -35,9 +35,9 @@ public GitHubService(ILogger logger) /// /// GitHub repository URL /// Contents fo the located CODEOWNERS file - public async Task> GetCodeownersFile(Uri repoUrl) + public async Task> GetCodeownersFile(Uri repoUrl) { - List result; + List result; if (codeownersFileCache.TryGetValue(repoUrl.ToString(), out result)) { return result; @@ -53,7 +53,7 @@ public async Task> GetCodeownersFile(Uri repoUrl) /// /// /// - private async Task> GetCodeownersFileImpl(Uri repoUrl) + private async Task> GetCodeownersFileImpl(Uri repoUrl) { // Gets the repo path from the URL var relevantPathParts = repoUrl.Segments.Skip(1).Take(2); @@ -64,7 +64,7 @@ private async Task> GetCodeownersFileImpl(Uri repoUrl) if (result.IsSuccessStatusCode) { logger.LogInformation("Retrieved CODEOWNERS file URL = {0}", codeOwnersUrl); - return CodeOwnersFile.ParseContent(await result.Content.ReadAsStringAsync()); + return CodeownersFile.GetCodeownersEntries(await result.Content.ReadAsStringAsync()); } logger.LogWarning("Could not retrieve CODEOWNERS file URL = {0} ResponseCode = {1}", codeOwnersUrl, result.StatusCode); diff --git a/tools/notification-configuration/notification-configuration.sln.DotSettings b/tools/notification-configuration/notification-configuration.sln.DotSettings index 1e20132c27c..8c1c0ec65f1 100644 --- a/tools/notification-configuration/notification-configuration.sln.DotSettings +++ b/tools/notification-configuration/notification-configuration.sln.DotSettings @@ -1,3 +1,4 @@  + AAD PR True \ No newline at end of file diff --git a/tools/notification-configuration/notification-creator/NotificationConfigurator.cs b/tools/notification-configuration/notification-creator/NotificationConfigurator.cs index 386e56a3f28..adecdc32fcb 100644 --- a/tools/notification-configuration/notification-creator/NotificationConfigurator.cs +++ b/tools/notification-configuration/notification-creator/NotificationConfigurator.cs @@ -209,8 +209,8 @@ private async Task SyncTeamWithCodeOwnerFile(BuildDefinition pipeline, WebApiTea logger.LogInformation("Searching CODEOWNERS for matching path for {0}", process.YamlFilename); - var codeOwnerEntry = CodeOwnersFile.FindOwnersForClosestMatch(codeOwnerEntries, process.YamlFilename); - codeOwnerEntry.FilterOutNonUserAliases(); + var codeOwnerEntry = CodeownersFile.GetMatchingCodeownersEntry(process.YamlFilename, codeOwnerEntries); + codeOwnerEntry.ExcludeNonUserAliases(); logger.LogInformation("Matching Contacts Path = {0}, NumContacts = {1}", process.YamlFilename, codeOwnerEntry.Owners.Count); diff --git a/tools/notification-configuration/notification-creator/YamlHelper.cs b/tools/notification-configuration/notification-creator/YamlHelper.cs index bdad08f876c..a8bf28802da 100644 --- a/tools/notification-configuration/notification-creator/YamlHelper.cs +++ b/tools/notification-configuration/notification-creator/YamlHelper.cs @@ -1,4 +1,4 @@ -using System; +using System; using YamlDotNet.Serialization; using YamlDotNet.Serialization.NamingConventions; @@ -6,10 +6,10 @@ namespace Azure.Sdk.Tools.NotificationConfiguration { static class YamlHelper { - private static ISerializer serializer = + private static readonly ISerializer serializer = new SerializerBuilder().WithNamingConvention(new CamelCaseNamingConvention()).Build(); - private static IDeserializer deserializer = + private static readonly IDeserializer deserializer = new DeserializerBuilder().WithNamingConvention(new CamelCaseNamingConvention()).Build();