From bf8d5c765ab52e000ab0a9cc4f90f2b14c1290fb Mon Sep 17 00:00:00 2001 From: Millicent Achieng Date: Fri, 2 Aug 2024 13:27:49 +0300 Subject: [PATCH] Implement conditional update of permissions tables based on HTML comment command (#282) * Return after exception parsing YAML metadata * Report when a set of request URLs do not have the same set of permissions * Report when the number of HTTP requests blocks are more than the number of permissions tables. * Implement additional properties in the permissions HTML comment * Update failing tests * Apply suggestions from code review Co-authored-by: Andrew Omondi * Update the comparison between newPermissionFileContents and requestPermissions to ignore case differences. --------- Co-authored-by: Andrew Omondi --- ApiDoctor.Console/Program.cs | 200 +++++++++++++----- .../YamlParserTests.cs | 2 +- ApiDoctor.Validation/CodeBlockAnnotation.cs | 18 ++ ApiDoctor.Validation/DocFile.cs | 7 +- 4 files changed, 175 insertions(+), 52 deletions(-) diff --git a/ApiDoctor.Console/Program.cs b/ApiDoctor.Console/Program.cs index 7f721b41..35be4533 100644 --- a/ApiDoctor.Console/Program.cs +++ b/ApiDoctor.Console/Program.cs @@ -2643,9 +2643,11 @@ private static async Task GeneratePermissionFilesAsync(GeneratePermissionF var originalFileContents = await File.ReadAllLinesAsync(docFile.FullPath); var parseStatus = PermissionsInsertionState.FindPermissionsHeader; int foundPermissionTablesOrBlocks = 0, foundHttpRequestBlocks = 0; - bool finishedParsing = false, isBootstrapped = false, ignorePermissionTableUpdate = false; + bool finishedParsing = false, isBootstrapped = false, ignorePermissionTableUpdate = false, + foundAllPermissionTables = false, mergePermissions = false; int insertionStartLine = -1, insertionEndLine = -1, httpRequestStartLine = -1, httpRequestEndLine = -1, - boilerplateStartLine = -1, boilerplateEndLine = -1, permissionsHeaderIndex = -1; + boilerplateStartLine = -1, boilerplateEndLine = -1, permissionsHeaderIndex = -1, codeBlockAnnotationEndLine = -1; + string[] requestUrlsForPermissions = null; for (var currentIndex = 0; currentIndex < originalFileContents.Length && !finishedParsing; currentIndex++) { var currentLine = originalFileContents[currentIndex].Trim(); @@ -2660,9 +2662,35 @@ private static async Task GeneratePermissionFilesAsync(GeneratePermissionF break; case PermissionsInsertionState.FindInsertionStartLine: if (currentLine.Contains("blockType", StringComparison.OrdinalIgnoreCase) && currentLine.Contains("\"ignored\"")) + { ignorePermissionTableUpdate = true; + } + + // Extract HTML comment + if (codeBlockAnnotationEndLine != -1 && (requestUrlsForPermissions != null || !mergePermissions)) + { + var htmlComment = insertionStartLine == codeBlockAnnotationEndLine + ? originalFileContents[insertionStartLine] + : string.Join(" ", originalFileContents.Skip(insertionStartLine).Take(codeBlockAnnotationEndLine + 1 - insertionStartLine)); + var metadataJsonString = DocFile.StripHtmlCommentTags(htmlComment); + var annotation = CodeBlockAnnotation.ParseMetadata(metadataJsonString); + requestUrlsForPermissions = annotation?.RequestUrls; + mergePermissions = annotation?.MergePermissions ?? false; + } - if (currentLine.Contains("[!INCLUDE [permissions-table](")) // bootstrapping already took place + if (currentLine.StartsWith("") || currentLine.Contains("}-->")) + { + codeBlockAnnotationEndLine = currentIndex; + } + } + else if (currentLine.Contains("} -->") || currentLine.Contains("}-->")) + { + codeBlockAnnotationEndLine = currentIndex; + } + else if (currentLine.Contains("[!INCLUDE [permissions-table](", StringComparison.OrdinalIgnoreCase)) // bootstrapping already took place { foundPermissionTablesOrBlocks++; if (ignorePermissionTableUpdate) @@ -2675,20 +2703,14 @@ private static async Task GeneratePermissionFilesAsync(GeneratePermissionF isBootstrapped = true; if (!options.BootstrappingOnly) { - // find the permissions block start line - for (var i = currentIndex; i > 0; i--) - { - if (originalFileContents[i].Contains("\r\n" + - $"[!INCLUDE [permissions-table](../{ReplaceWindowsByLinuxPathSeparators(Path.Combine(permissionsFileRelativePath, permissionsFileName))})]"; + var permissionsBlockText = GeneratePermissionsBlockText(docFileName, Path.Combine(permissionsFileRelativePath, permissionsFileName), requestUrlsForPermissions, mergePermissions); IEnumerable updatedFileContents = originalFileContents; updatedFileContents = updatedFileContents.Splice(insertionStartLine, insertionEndLine + 1 - insertionStartLine); updatedFileContents = FileSplicer(updatedFileContents.ToArray(), insertionStartLine - 1, permissionsBlockText); @@ -2881,13 +2918,17 @@ private static async Task GeneratePermissionFilesAsync(GeneratePermissionF case PermissionsInsertionState.FindNextPermissionBlock: if (!ignorePermissionTableUpdate) { + var newFileContents = await File.ReadAllLinesAsync(docFile.FullPath); + currentIndex = newFileContents.Length == originalFileContents.Length + ? insertionEndLine + : codeBlockAnnotationEndLine - (originalFileContents.Length - newFileContents.Length) + 1; + originalFileContents = newFileContents; + insertionStartLine = insertionEndLine = httpRequestStartLine = httpRequestEndLine = codeBlockAnnotationEndLine = - 1; + mergePermissions = false; + requestUrlsForPermissions = null; foundHttpRequestBlocks = 0; - currentIndex = insertionStartLine + 2; - originalFileContents = await File.ReadAllLinesAsync(docFile.FullPath); - insertionStartLine = insertionEndLine = httpRequestStartLine = httpRequestEndLine = -1; } - isBootstrapped = false; - ignorePermissionTableUpdate = false; + isBootstrapped = ignorePermissionTableUpdate = false; parseStatus = PermissionsInsertionState.FindInsertionStartLine; break; default: @@ -2936,29 +2977,92 @@ private static string ConvertToThreeColumnPermissionsTable(IEnumerable t return tableString.ToString(); } - private static string GetPermissionsMarkdownTableForHttpRequestBlock(List httpRequests, PermissionsDocument permissionsDoc) + private static string GeneratePermissionsBlockText(string docFileName, string permissionsIncludeFilePath, string[] requestUrlsForPermissions, bool mergePermissions) { - // use the first HTTP Request, we are assuming the group of URLs will have the same set of permissions - var request = httpRequests.Where(x => !string.IsNullOrWhiteSpace(x)).FirstOrDefault(); - if (HttpParser.TryParseHttpRequest(request, out var parsedRequest)) + var newLineSpaces = requestUrlsForPermissions != null ? $"{Environment.NewLine} " : string.Empty; + + string requestUrlsJson = requestUrlsForPermissions != null + ? $", {newLineSpaces}\"requestUrls\": [\"{string.Join("\", \"", requestUrlsForPermissions)}\"]" + : string.Empty; + + string mergePermissionsJson = mergePermissions + ? $",{newLineSpaces}\"mergePermissions\": true" + : string.Empty; + + + var permissionsBlockText = $"\r\n" + + $"[!INCLUDE [permissions-table](../{ReplaceWindowsByLinuxPathSeparators(permissionsIncludeFilePath)})]"; + + return permissionsBlockText; + } + + private static string GetPermissionsMarkdownTableForHttpRequestBlock(PermissionsDocument permissionsDocument, IEnumerable httpRequests, bool mergePermissions, + string docFileName, int permissionsTablePosition) + { + var requestPaths = new List(); + + // check validity of request paths + foreach (var request in httpRequests) { - try + if (!HttpParser.TryParseHttpRequest(request, out var parsedRequest)) { - // remove $ref, $count, $value segments from paths - parsedRequest.Url = Constants.QueryOptionSegementRegex.Replace(parsedRequest.Url, string.Empty).TrimEnd('/').ToLowerInvariant(); + FancyConsole.WriteLine($"Malformed HTTP request '{request}' in {docFileName} for permissions table ({permissionsTablePosition})"); + continue; + } + // remove $ref, $count, $value segments from paths + parsedRequest.Url = Constants.QueryOptionSegementRegex.Replace(parsedRequest.Url, string.Empty).TrimEnd('/').ToLowerInvariant(); - // normalize function parameters - parsedRequest.Url = Constants.FunctionParameterRegex.Replace(parsedRequest.Url, "{value}"); + // normalize function parameters + parsedRequest.Url = Constants.FunctionParameterRegex.Replace(parsedRequest.Url, "{value}"); - var generator = new PermissionsStubGenerator(permissionsDoc, parsedRequest.Url, parsedRequest.Method, false, true); + requestPaths.Add(parsedRequest); + } + if (requestPaths.Count == 0) + { + return null; + } + + // fetch merged permissions table for request URLs + var concatenatedRequests = string.Join(";", requestPaths.Select(x => x.Url)); + if (mergePermissions) + { + try + { + var generator = new PermissionsStubGenerator(permissionsDocument, concatenatedRequests, requestPaths[0].Method, false, true) + { + MergeMultiplePaths = true + }; return generator.GenerateTable(); } catch (Exception ex) { - FancyConsole.WriteLine($"Could not fetch permissions for '{parsedRequest.Method} {parsedRequest.Url}': {ex.Message}"); + FancyConsole.WriteLine($"Could not fetch permissions for '{requestPaths[0].Method} {concatenatedRequests}': {ex.Message}"); + return null; + } + } + + // fetch permissions table for individual request URL + string newPermissionFileContents = null; + foreach (var request in requestPaths) + { + try + { + var generator = new PermissionsStubGenerator(permissionsDocument, request.Url, request.Method, false, true); + var requestPermissions = generator.GenerateTable(); + newPermissionFileContents ??= requestPermissions; + + if (!newPermissionFileContents.Equals(requestPermissions, StringComparison.OrdinalIgnoreCase)) + { + FancyConsole.WriteLine(ConsoleColor.Yellow, $"Encountered request URL(s) for permissions table ({permissionsTablePosition}) in {docFileName} with a different set of permissions"); + } + } + catch (Exception ex) + { + FancyConsole.WriteLine($"Could not fetch permissions for '{request.Method} {request.Url}': {ex.Message}"); + return null; } } - return null; + return newPermissionFileContents; } private static async Task GetPermissionsDocumentAsync(string filePathOrUrl) diff --git a/ApiDoctor.Validation.UnitTests/YamlParserTests.cs b/ApiDoctor.Validation.UnitTests/YamlParserTests.cs index 96c4a11d..11a568c9 100644 --- a/ApiDoctor.Validation.UnitTests/YamlParserTests.cs +++ b/ApiDoctor.Validation.UnitTests/YamlParserTests.cs @@ -85,7 +85,7 @@ public void MalformedYamlGeneratesError() var error = issues.Issues.FirstOrDefault(); Assert.That(error != null); Assert.That(error.IsError); - Assert.That(error.Message == "Incorrect YAML header format"); + Assert.That(error.Message.FirstLineOnly() == "Incorrect YAML header format"); } } } \ No newline at end of file diff --git a/ApiDoctor.Validation/CodeBlockAnnotation.cs b/ApiDoctor.Validation/CodeBlockAnnotation.cs index 316b120e..a0349f8b 100644 --- a/ApiDoctor.Validation/CodeBlockAnnotation.cs +++ b/ApiDoctor.Validation/CodeBlockAnnotation.cs @@ -302,6 +302,19 @@ public string ResourceType [JsonProperty("keyProperty", DefaultValueHandling=DefaultValueHandling.Ignore)] public string KeyPropertyName { get; set; } + /// + /// Request URLs to use to populate permissions table. + /// + [JsonProperty("requestUrls", DefaultValueHandling = DefaultValueHandling.Ignore)] + public string[] RequestUrls { get; set; } + + /// + /// Instruction to merge permissions of requestUrls into one table. + /// If false, there'll be an error if requestUrls do not share the same set of permissions. + /// + [JsonProperty("mergePermissions", DefaultValueHandling=DefaultValueHandling.Ignore)] + public bool MergePermissions { get; set; } + /// /// Use this property to declare that a custom function request is idempotent (has no side-effects). /// @@ -571,5 +584,10 @@ public enum CodeBlockType /// A block representing a test parameter definition for the preceding example /// TestParams, + + /// + /// Permissions code block. A block representing parameters to use to populate permissions table + /// + Permissions } } diff --git a/ApiDoctor.Validation/DocFile.cs b/ApiDoctor.Validation/DocFile.cs index 64bc8a11..16cb313a 100644 --- a/ApiDoctor.Validation/DocFile.cs +++ b/ApiDoctor.Validation/DocFile.cs @@ -397,9 +397,10 @@ internal static void ParseYamlMetadata(string yamlMetadata, IssueLogger issues) { dictionary = yamlDeserializer.Deserialize>(yamlMetadata); } - catch (Exception) + catch (Exception ex) { - issues.Error(ValidationErrorCode.IncorrectYamlHeaderFormat, "Incorrect YAML header format"); + issues.Error(ValidationErrorCode.IncorrectYamlHeaderFormat, "Incorrect YAML header format", ex); + return; } List missingHeaders = new List(); @@ -1110,7 +1111,7 @@ private enum TabDetectionState /// /// /// - private static string StripHtmlCommentTags(string content) + public static string StripHtmlCommentTags(string content) { const string startComment = "";